diff --git a/internal/dataplane/kong_client.go b/internal/dataplane/kong_client.go index a73561799b..9e03f6dd6e 100644 --- a/internal/dataplane/kong_client.go +++ b/internal/dataplane/kong_client.go @@ -11,6 +11,8 @@ import ( "github.com/kong/go-kong/kong" "github.com/prometheus/client_golang/prometheus" "github.com/sirupsen/logrus" + corev1 "k8s.io/api/core/v1" + "k8s.io/client-go/tools/record" "sigs.k8s.io/controller-runtime/pkg/client" "github.com/kong/kubernetes-ingress-controller/v2/internal/dataplane/deckgen" @@ -106,6 +108,7 @@ type KongClient struct { // whether a Kubernetes object has corresponding data-plane configuration that // is actively configured (e.g. to know how to set the object status). kubernetesObjectReportsFilter k8sobj.Set + eventsRecorder record.EventRecorder } // NewKongClient provides a new KongClient object after connecting to the @@ -118,6 +121,7 @@ func NewKongClient( skipCACertificates bool, diagnostic util.ConfigDumpDiagnostic, kongConfig sendconfig.Kong, + eventsRecorder record.EventRecorder, ) (*KongClient, error) { // build the client object cache := store.NewCacheStores() @@ -131,6 +135,7 @@ func NewKongClient( prometheusMetrics: metrics.NewCtrlFuncMetrics(), cache: &cache, kongConfig: kongConfig, + eventsRecorder: eventsRecorder, } // download the kong root configuration (and validate connectivity to the proxy API) @@ -312,12 +317,18 @@ func (c *KongClient) Update(ctx context.Context) error { // parse the Kubernetes objects from the storer into Kong configuration kongstate := p.Build() - // todo: does it still make sense to report TranslationCount when Build no longer returns an error? - // https://github.com/Kong/kubernetes-ingress-controller/issues/1892 - c.prometheusMetrics.TranslationCount.With(prometheus.Labels{ - metrics.SuccessKey: metrics.SuccessTrue, - }).Inc() - c.logger.Debug("successfully built data-plane configuration") + if errors := p.PopParsingErrors(); errors != nil { + c.createParsingErrorsEvents(errors) + c.prometheusMetrics.TranslationCount.With(prometheus.Labels{ + metrics.SuccessKey: metrics.SuccessFalse, + }).Inc() + c.logger.Debugf("%d translation errors occurred when building data-plane configuration", len(errors)) + } else { + c.prometheusMetrics.TranslationCount.With(prometheus.Labels{ + metrics.SuccessKey: metrics.SuccessTrue, + }).Inc() + c.logger.Debug("successfully built data-plane configuration") + } // generate the deck configuration to be applied to the admin API c.logger.Debug("converting configuration to deck config") @@ -438,3 +449,12 @@ func (c *KongClient) updateKubernetesObjectReportFilter(set k8sobj.Set) { defer c.kubernetesObjectReportLock.Unlock() c.kubernetesObjectReportsFilter = set } + +func (c *KongClient) createParsingErrorsEvents(errors []parser.ParsingError) { + const reason = "TranslationToKongConfigurationFailed" + for _, err := range errors { + for _, obj := range err.RelatedObjects() { + c.eventsRecorder.Event(obj, corev1.EventTypeWarning, reason, err.Reason()) + } + } +} diff --git a/internal/dataplane/parser/parser.go b/internal/dataplane/parser/parser.go index d6c617aca9..b1976bb7f5 100644 --- a/internal/dataplane/parser/parser.go +++ b/internal/dataplane/parser/parser.go @@ -39,6 +39,41 @@ const ( // Parser - Public Types // ----------------------------------------------------------------------------- +type ParsingError struct { + relatedObjects []client.Object + reason string +} + +func NewParsingError(reason string, relatedObjects ...client.Object) ParsingError { + if reason == "" { + reason = "unknown" + } + return ParsingError{ + relatedObjects: relatedObjects, + reason: reason, + } +} + +func (p ParsingError) RelatedObjects() []client.Object { + return p.relatedObjects +} + +func (p ParsingError) Reason() string { + return p.reason +} + +type parsingErrorsCollector struct { + errors []ParsingError +} + +func newParsingErrorsCollector() *parsingErrorsCollector { + return &parsingErrorsCollector{} +} + +func (c *parsingErrorsCollector) ParsingError(reason string, relatedObjects ...client.Object) { + c.errors = append(c.errors, NewParsingError(reason, relatedObjects...)) +} + // Parser parses Kubernetes objects and configurations into their // equivalent Kong objects and configurations, producing a complete // state configuration for the Kong Admin API. @@ -51,6 +86,7 @@ type Parser struct { featureEnabledCombinedServiceRoutes bool flagEnabledRegexPathPrefix bool + errorsCollector *parsingErrorsCollector } // NewParser produces a new Parser object provided a logging mechanism @@ -60,8 +96,9 @@ func NewParser( storer store.Storer, ) *Parser { return &Parser{ - logger: logger, - storer: storer, + logger: logger, + storer: storer, + errorsCollector: newParsingErrorsCollector(), } } @@ -118,7 +155,7 @@ func (p *Parser) Build() *kongstate.KongState { 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(p.logger, p.storer) return &result } @@ -154,6 +191,13 @@ func (p *Parser) GenerateKubernetesObjectReport() []client.Object { return report } +// PopParsingErrors pops all the parsing errors collected during the last parsing round. +func (p *Parser) PopParsingErrors() []ParsingError { + errors := p.errorsCollector.errors + p.errorsCollector.errors = nil + return errors +} + // ----------------------------------------------------------------------------- // Parser - Public Methods - Other Optional Features // ----------------------------------------------------------------------------- diff --git a/internal/dataplane/parser/translate_secrets.go b/internal/dataplane/parser/translate_secrets.go index 7121a34e49..35ce6f4bc7 100644 --- a/internal/dataplane/parser/translate_secrets.go +++ b/internal/dataplane/parser/translate_secrets.go @@ -2,23 +2,26 @@ 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() +func (p *Parser) getCACerts() []kong.CACertificate { + log := p.logger + caCertSecrets, err := p.storer.ListCACerts() if err != nil { log.WithError(err).Error("failed to list CA certs") return nil @@ -26,22 +29,18 @@ func getCACerts(log logrus.FieldLogger, storer store.Storer, plugins []kongstate 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.errorsCollector.ParsingError("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") + affectedObjects := getPluginsAssociatedWithCACertSecret(secretID, p.storer) + affectedObjects = append(affectedObjects, certSecret) + p.errorsCollector.ParsingError(fmt.Sprintf("invalid CA certificate: %s", err), affectedObjects...) continue } @@ -77,19 +76,17 @@ 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 } @@ -97,10 +94,15 @@ func getPluginsAssociatedWithCACertSecret(plugins []kongstate.Plugin, secretID s 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()) } } diff --git a/internal/dataplane/parser/translate_secrets_test.go b/internal/dataplane/parser/translate_secrets_test.go index f3b03e3d56..896ffb17a0 100644 --- a/internal/dataplane/parser/translate_secrets_test.go +++ b/internal/dataplane/parser/translate_secrets_test.go @@ -4,14 +4,14 @@ import ( "testing" "github.com/kong/go-kong/kong" - "github.com/stretchr/testify/require" "github.com/kong/kubernetes-ingress-controller/v2/internal/dataplane/kongstate" ) func TestGetPluginsAssociatedWithCACertSecret(t *testing.T) { secretID := "8a3753e0-093b-43d9-9d39-27985c987d92" //nolint:gosec - plugins := []kongstate.Plugin{ + // todo: adapt to implementation + _ = []kongstate.Plugin{ { Plugin: kong.Plugin{ Name: kong.String("associated-plugin"), @@ -35,6 +35,6 @@ func TestGetPluginsAssociatedWithCACertSecret(t *testing.T) { }, } - associatedPlugins := getPluginsAssociatedWithCACertSecret(plugins, secretID) - require.ElementsMatch(t, []string{"associated-plugin", "another-associated-plugin"}, associatedPlugins) + // associatedPlugins := getPluginsAssociatedWithCACertSecret(plugins, secretID) + // require.ElementsMatch(t, []string{"associated-plugin", "another-associated-plugin"}, associatedPlugins) } diff --git a/internal/manager/run.go b/internal/manager/run.go index ca0d4177b8..bb92bad4c6 100644 --- a/internal/manager/run.go +++ b/internal/manager/run.go @@ -144,7 +144,18 @@ func Run(ctx context.Context, c *Config, diagnostic util.ConfigDumpDiagnostic) e if err != nil { return fmt.Errorf("%f is not a valid number of seconds to the timeout config for the kong client: %w", c.ProxyTimeoutSeconds, err) } - dataplaneClient, err := dataplane.NewKongClient(deprecatedLogger, timeoutDuration, c.IngressClassName, c.EnableReverseSync, c.SkipCACertificates, diagnostic, kongConfig) + + dataplaneEventRecorder := mgr.GetEventRecorderFor("kubernetes-ingress-controller-data-plane") + dataplaneClient, err := dataplane.NewKongClient( + deprecatedLogger, + timeoutDuration, + c.IngressClassName, + c.EnableReverseSync, + c.SkipCACertificates, + diagnostic, + kongConfig, + dataplaneEventRecorder, + ) if err != nil { return fmt.Errorf("failed to initialize kong data-plane client: %w", err) } diff --git a/internal/store/store.go b/internal/store/store.go index 164987c141..2f6e310bb6 100644 --- a/internal/store/store.go +++ b/internal/store/store.go @@ -96,6 +96,8 @@ type Storer interface { ListTCPIngresses() ([]*kongv1beta1.TCPIngress, error) ListUDPIngresses() ([]*kongv1beta1.UDPIngress, error) ListKnativeIngresses() ([]*knative.Ingress, error) + ListKongPlugins() []*kongv1.KongPlugin + ListKongClusterPlugins() []*kongv1.KongClusterPlugin ListGlobalKongPlugins() ([]*kongv1.KongPlugin, error) ListGlobalKongClusterPlugins() ([]*kongv1.KongClusterPlugin, error) ListKongConsumers() []*kongv1.KongConsumer @@ -961,6 +963,28 @@ func (s Store) ListGlobalKongClusterPlugins() ([]*kongv1.KongClusterPlugin, erro return plugins, nil } +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 +} + +func (s Store) ListKongPlugins() []*kongv1.KongPlugin { + var plugins []*kongv1.KongPlugin + for _, item := range s.stores.Plugin.List() { + p, ok := item.(*kongv1.KongPlugin) + if ok && s.isValidIngressClass(&p.ObjectMeta, annotations.IngressClassKey, s.getIngressClassHandling()) { + 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) {