feat(parser): propagate CA certificate failures (#3114)

Refactor the logging for plugin failures related to CA certificates.
      
Forward CA certificate failures to the failure collector, to attach events to 
affected objects.

Co-authored-by: Travis Raines <[email protected]>

Author: czeslavo

internal/dataplane/parser/parser.go

@@ -126,7 +126,7 @@ func (p *Parser) Build() (*kongstate.KongState, []TranslationFailure) {
 	result.Certificates = mergeCerts(p.logger, ingressCerts, gatewayCerts)
 
 	// populate CA certificates in Kong
-	result.CACertificates = getCACerts(p.logger, p.storer, result.Plugins)
+	result.CACertificates = p.getCACerts()
 
 	return &result, p.popTranslationFailures()
 }
@@ -182,14 +182,32 @@ func (p *Parser) EnableRegexPathPrefix() {
 	p.flagEnabledRegexPathPrefix = true
 }
 
-func (p *Parser) popTranslationFailures() []TranslationFailure {
-	return p.failuresCollector.PopTranslationFailures()
-}
-
 // -----------------------------------------------------------------------------
 // Parser - Private Methods
 // -----------------------------------------------------------------------------
 
+// registerTranslationFailure should be called when any Kubernetes object translation failure is encountered.
+func (p *Parser) registerTranslationFailure(reason string, causingObjects ...client.Object) {
+	p.failuresCollector.PushTranslationFailure(reason, causingObjects...)
+	p.logTranslationFailure(reason, causingObjects...)
+}
+
+// logTranslationFailure logs an error message signaling that a translation error has occurred along with its reason
+// for every causing object.
+func (p *Parser) logTranslationFailure(reason string, causingObjects ...client.Object) {
+	for _, obj := range causingObjects {
+		p.logger.WithFields(logrus.Fields{
+			"name":      obj.GetName(),
+			"namespace": obj.GetNamespace(),
+			"GVK":       obj.GetObjectKind().GroupVersionKind().String(),
+		}).Errorf("translation failed: %s", reason)
+	}
+}
+
+func (p *Parser) popTranslationFailures() []TranslationFailure {
+	return p.failuresCollector.PopTranslationFailures()
+}
+
 func knativeIngressToNetworkingTLS(tls []knative.IngressTLS) []netv1beta1.IngressTLS {
 	var result []netv1beta1.IngressTLS
 

internal/dataplane/parser/parser_test.go

@@ -920,7 +920,7 @@ func TestCACertificate(t *testing.T) {
 		secrets := []*corev1.Secret{
 			{
 				ObjectMeta: metav1.ObjectMeta{
-					Name:      "foo",
+					Name:      "valid-cert",
 					Namespace: "default",
 					Labels: map[string]string{
 						"konghq.com/ca-cert": "true",
@@ -936,7 +936,7 @@ func TestCACertificate(t *testing.T) {
 			},
 			{
 				ObjectMeta: metav1.ObjectMeta{
-					Name:      "bar",
+					Name:      "missing-cert-key",
 					Namespace: "non-default",
 					Labels: map[string]string{
 						"konghq.com/ca-cert": "true",
@@ -952,7 +952,7 @@ func TestCACertificate(t *testing.T) {
 			},
 			{
 				ObjectMeta: metav1.ObjectMeta{
-					Name:      "baz",
+					Name:      "missing-id-key",
 					Namespace: "non-default",
 					Labels: map[string]string{
 						"konghq.com/ca-cert": "true",
@@ -968,7 +968,7 @@ func TestCACertificate(t *testing.T) {
 			},
 			{
 				ObjectMeta: metav1.ObjectMeta{
-					Name:      "baz",
+					Name:      "expired-cert",
 					Namespace: "non-default",
 					Labels: map[string]string{
 						"konghq.com/ca-cert": "true",
@@ -990,7 +990,7 @@ func TestCACertificate(t *testing.T) {
 		assert.Nil(err)
 		p := mustNewParser(t, store)
 		state, translationFailures := p.Build()
-		require.Empty(t, translationFailures)
+		assert.Len(translationFailures, 3)
 		assert.NotNil(state)
 
 		assert.Equal(1, len(state.CACertificates))

internal/dataplane/parser/translate_secrets.go

@@ -2,46 +2,43 @@ package parser
 
 import (
 	"crypto/x509"
+	"encoding/json"
 	"encoding/pem"
 	"errors"
+	"fmt"
 	"time"
 
 	"github.com/kong/go-kong/kong"
-	"github.com/sirupsen/logrus"
 	corev1 "k8s.io/api/core/v1"
+	v1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
+	"sigs.k8s.io/controller-runtime/pkg/client"
 
-	"github.com/kong/kubernetes-ingress-controller/v2/internal/dataplane/kongstate"
 	"github.com/kong/kubernetes-ingress-controller/v2/internal/store"
 )
 
 // getCACerts translates CA certificates Secrets to kong.CACertificates. It ensures every certificate's structure and
-// validity. In case of violation of any validation rule, a secret gets skipped in a result and error message is logged
-// with affected plugins for context.
-func getCACerts(log logrus.FieldLogger, storer store.Storer, plugins []kongstate.Plugin) []kong.CACertificate {
-	caCertSecrets, err := storer.ListCACerts()
+// validity. It skips Secrets that do not contain a valid certificate and reports translation failures for them.
+func (p *Parser) getCACerts() []kong.CACertificate {
+	caCertSecrets, err := p.storer.ListCACerts()
 	if err != nil {
-		log.WithError(err).Error("failed to list CA certs")
+		p.logger.WithError(err).Error("failed to list CA certs")
 		return nil
 	}
 
 	var caCerts []kong.CACertificate
 	for _, certSecret := range caCertSecrets {
-		log := log.WithFields(logrus.Fields{
-			"secret_name":      certSecret.Name,
-			"secret_namespace": certSecret.Namespace,
-		})
-
 		idBytes, ok := certSecret.Data["id"]
 		if !ok {
-			log.Error("skipping synchronisation, invalid CA certificate: missing 'id' field in data")
+			p.registerTranslationFailure("invalid CA certificate: missing 'id' field in data", certSecret)
 			continue
 		}
 		secretID := string(idBytes)
 
 		caCert, err := toKongCACertificate(certSecret, secretID)
 		if err != nil {
-			logWithAffectedPlugins(log, plugins, secretID).WithError(err).
-				Error("skipping synchronisation, invalid CA certificate")
+			relatedObjects := getPluginsAssociatedWithCACertSecret(secretID, p.storer)
+			relatedObjects = append(relatedObjects, certSecret.DeepCopy())
+			p.registerTranslationFailure(fmt.Sprintf("invalid CA certificate: %s", err), relatedObjects...)
 			continue
 		}
 
@@ -77,30 +74,33 @@ func toKongCACertificate(certSecret *corev1.Secret, secretID string) (kong.CACer
 	}, nil
 }
 
-func logWithAffectedPlugins(log logrus.FieldLogger, plugins []kongstate.Plugin, secretID string) logrus.FieldLogger {
-	affectedPlugins := getPluginsAssociatedWithCACertSecret(plugins, secretID)
-	return log.WithField("affected_plugins", affectedPlugins)
-}
-
-func getPluginsAssociatedWithCACertSecret(plugins []kongstate.Plugin, secretID string) []string {
-	refersToSecret := func(pluginConfig map[string]interface{}) bool {
-		caCertReferences, ok := pluginConfig["ca_certificates"].([]string)
-		if !ok {
+func getPluginsAssociatedWithCACertSecret(secretID string, storer store.Storer) []client.Object {
+	refersToSecret := func(pluginConfig v1.JSON) bool {
+		cfg := struct {
+			CACertificates []string `json:"ca_certificates,omitempty"`
+		}{}
+		err := json.Unmarshal(pluginConfig.Raw, &cfg)
+		if err != nil {
 			return false
 		}
 
-		for _, reference := range caCertReferences {
+		for _, reference := range cfg.CACertificates {
 			if reference == secretID {
 				return true
 			}
 		}
 		return false
 	}
 
-	var affectedPlugins []string
-	for _, p := range plugins {
-		if refersToSecret(p.Config) && p.Name != nil {
-			affectedPlugins = append(affectedPlugins, *p.Name)
+	var affectedPlugins []client.Object
+	for _, p := range storer.ListKongPlugins() {
+		if refersToSecret(p.Config) {
+			affectedPlugins = append(affectedPlugins, p.DeepCopy())
+		}
+	}
+	for _, p := range storer.ListKongClusterPlugins() {
+		if refersToSecret(p.Config) {
+			affectedPlugins = append(affectedPlugins, p.DeepCopy())
 		}
 	}
 

internal/dataplane/parser/translate_secrets_test.go

@@ -1,40 +1,60 @@
 package parser
 
 import (
+	"fmt"
 	"testing"
 
-	"github.com/kong/go-kong/kong"
 	"github.com/stretchr/testify/require"
+	v1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"sigs.k8s.io/controller-runtime/pkg/client"
 
-	"github.com/kong/kubernetes-ingress-controller/v2/internal/dataplane/kongstate"
+	"github.com/kong/kubernetes-ingress-controller/v2/internal/annotations"
+	"github.com/kong/kubernetes-ingress-controller/v2/internal/store"
+	kongv1 "github.com/kong/kubernetes-ingress-controller/v2/pkg/apis/configuration/v1"
 )
 
 func TestGetPluginsAssociatedWithCACertSecret(t *testing.T) {
-	secretID := "8a3753e0-093b-43d9-9d39-27985c987d92" //nolint:gosec
-	plugins := []kongstate.Plugin{
-		{
-			Plugin: kong.Plugin{
-				Name: kong.String("associated-plugin"),
-				Config: map[string]interface{}{
-					"ca_certificates": []string{secretID},
-				},
+	kongPluginWithSecret := func(name, secretID string) *kongv1.KongPlugin {
+		return &kongv1.KongPlugin{
+			ObjectMeta: metav1.ObjectMeta{
+				Name: name,
 			},
-		},
-		{
-			Plugin: kong.Plugin{
-				Name: kong.String("another-associated-plugin"),
-				Config: map[string]interface{}{
-					"ca_certificates": []string{secretID},
-				},
+			Config: v1.JSON{
+				Raw: []byte(fmt.Sprintf(`{"ca_certificates": ["%s"]}`, secretID)),
 			},
-		},
-		{
-			Plugin: kong.Plugin{
-				Name: kong.String("non-associated-plugin"),
+		}
+	}
+	kongClusterPluginWithSecret := func(name, secretID string) *kongv1.KongClusterPlugin {
+		return &kongv1.KongClusterPlugin{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:        name,
+				Annotations: map[string]string{annotations.IngressClassKey: annotations.DefaultIngressClass},
+			},
+			Config: v1.JSON{
+				Raw: []byte(fmt.Sprintf(`{"ca_certificates": ["%s"]}`, secretID)),
 			},
-		},
+		}
 	}
 
-	associatedPlugins := getPluginsAssociatedWithCACertSecret(plugins, secretID)
-	require.ElementsMatch(t, []string{"associated-plugin", "another-associated-plugin"}, associatedPlugins)
+	//nolint:gosec
+	const (
+		secretID        = "8a3753e0-093b-43d9-9d39-27985c987d92"
+		anotherSecretID = "99fa09c7-f849-4449-891e-19b9a0015763"
+	)
+	var (
+		associatedPlugin           = kongPluginWithSecret("associated_plugin", secretID)
+		nonAssociatedPlugin        = kongPluginWithSecret("non_associated_plugin", anotherSecretID)
+		associatedClusterPlugin    = kongClusterPluginWithSecret("associated_cluster_plugin", secretID)
+		nonAssociatedClusterPlugin = kongClusterPluginWithSecret("non_associated_cluster_plugin", anotherSecretID)
+	)
+	storer, err := store.NewFakeStore(store.FakeObjects{
+		KongPlugins:        []*kongv1.KongPlugin{associatedPlugin, nonAssociatedPlugin},
+		KongClusterPlugins: []*kongv1.KongClusterPlugin{associatedClusterPlugin, nonAssociatedClusterPlugin},
+	})
+	require.NoError(t, err)
+
+	gotPlugins := getPluginsAssociatedWithCACertSecret(secretID, storer)
+	expectedPlugins := []client.Object{associatedPlugin, associatedClusterPlugin}
+	require.ElementsMatch(t, expectedPlugins, gotPlugins, "expected plugins do not match actual ones")
 }

internal/store/store.go

@@ -98,6 +98,8 @@ type Storer interface {
 	ListKnativeIngresses() ([]*knative.Ingress, error)
 	ListGlobalKongPlugins() ([]*kongv1.KongPlugin, error)
 	ListGlobalKongClusterPlugins() ([]*kongv1.KongClusterPlugin, error)
+	ListKongPlugins() []*kongv1.KongPlugin
+	ListKongClusterPlugins() []*kongv1.KongClusterPlugin
 	ListKongConsumers() []*kongv1.KongConsumer
 	ListCACerts() ([]*corev1.Secret, error)
 }
@@ -920,7 +922,6 @@ func (s Store) ListKongConsumers() []*kongv1.KongConsumer {
 // This function remains only to provide warnings to users with old configuration.
 func (s Store) ListGlobalKongPlugins() ([]*kongv1.KongPlugin, error) {
 	var plugins []*kongv1.KongPlugin
-	// var globalPlugins []*configurationv1.KongPlugin
 	req, err := labels.NewRequirement("global", selection.Equals, []string{"true"})
 	if err != nil {
 		return nil, err
@@ -963,6 +964,30 @@ func (s Store) ListGlobalKongClusterPlugins() ([]*kongv1.KongClusterPlugin, erro
 	return plugins, nil
 }
 
+// ListKongClusterPlugins lists all KongClusterPlugins that match expected ingress.class annotation.
+func (s Store) ListKongClusterPlugins() []*kongv1.KongClusterPlugin {
+	var plugins []*kongv1.KongClusterPlugin
+	for _, item := range s.stores.ClusterPlugin.List() {
+		p, ok := item.(*kongv1.KongClusterPlugin)
+		if ok && s.isValidIngressClass(&p.ObjectMeta, annotations.IngressClassKey, s.getIngressClassHandling()) {
+			plugins = append(plugins, p)
+		}
+	}
+	return plugins
+}
+
+// ListKongPlugins lists all KongPlugins.
+func (s Store) ListKongPlugins() []*kongv1.KongPlugin {
+	var plugins []*kongv1.KongPlugin
+	for _, item := range s.stores.Plugin.List() {
+		p, ok := item.(*kongv1.KongPlugin)
+		if ok {
+			plugins = append(plugins, p)
+		}
+	}
+	return plugins
+}
+
 // ListCACerts returns all Secrets containing the label
 // "konghq.com/ca-cert"="true".
 func (s Store) ListCACerts() ([]*corev1.Secret, error) {