From bf5fb880ef68a1644da854791b5eae8c98b907a2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Grzegorz=20Burzy=C5=84ski?= Date: Thu, 3 Nov 2022 15:05:59 +0100 Subject: [PATCH] feat(#3097): create warning translation failure events for CA secrets (#3125) It starts creating Kubernetes Events for all TranslationFailures detected during the translation phase. It will create one event per causing object. An integration test suite is also added in order to track all translation failure cases - which should make eventual future refactors safer (e.g. moving validation rules from the parser to the controllers). --- CHANGELOG.md | 2 + internal/dataplane/kong_client.go | 21 +++ internal/manager/consts.go | 3 + internal/manager/run.go | 15 +- test/integration/translation_failures_test.go | 130 ++++++++++++++++++ 5 files changed, 169 insertions(+), 2 deletions(-) create mode 100644 test/integration/translation_failures_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index 704ca64039..f76e405e5c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -110,6 +110,8 @@ Adding a new version? You'll need three changes: [#3121](https://github.com/Kong/kubernetes-ingress-controller/pull/3121) - Routes support annotations for path handling. [#3121](https://github.com/Kong/kubernetes-ingress-controller/pull/3121) +- Warning events are recorded when CA secrets cannot be properly translated into Kong configuration. + [#3125](https://github.com/Kong/kubernetes-ingress-controller/pull/3125) ### Fixed diff --git a/internal/dataplane/kong_client.go b/internal/dataplane/kong_client.go index 8e9b21300f..0a8399a4b7 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" @@ -24,6 +26,9 @@ import ( "github.com/kong/kubernetes-ingress-controller/v2/internal/versions" ) +// KongConfigurationTranslationFailedEventReason defines an event reason used for creating all translation failure events. +const KongConfigurationTranslationFailedEventReason = "KongConfigurationTranslationFailed" + // ----------------------------------------------------------------------------- // Dataplane Client - Kong - Public Types // ----------------------------------------------------------------------------- @@ -106,6 +111,9 @@ 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 + + // eventRecorder is used to record warning events for translation failures. + eventRecorder record.EventRecorder } // NewKongClient provides a new KongClient object after connecting to the @@ -118,6 +126,7 @@ func NewKongClient( skipCACertificates bool, diagnostic util.ConfigDumpDiagnostic, kongConfig sendconfig.Kong, + eventRecorder record.EventRecorder, ) (*KongClient, error) { // build the client object cache := store.NewCacheStores() @@ -131,6 +140,7 @@ func NewKongClient( prometheusMetrics: metrics.NewCtrlFuncMetrics(), cache: &cache, kongConfig: kongConfig, + eventRecorder: eventRecorder, } // download the kong root configuration (and validate connectivity to the proxy API) @@ -320,6 +330,7 @@ func (c *KongClient) Update(ctx context.Context) error { c.prometheusMetrics.TranslationCount.With(prometheus.Labels{ metrics.SuccessKey: metrics.SuccessFalse, }).Inc() + c.recordTranslationFailureWarningEvents(translationFailures) c.logger.Debugf("%d translation failures have occurred when building data-plane configuration", failuresCount) } else { c.prometheusMetrics.TranslationCount.With(prometheus.Labels{ @@ -447,3 +458,13 @@ func (c *KongClient) updateKubernetesObjectReportFilter(set k8sobj.Set) { defer c.kubernetesObjectReportLock.Unlock() c.kubernetesObjectReportsFilter = set } + +// recordTranslationFailureWarningEvents records a warning KongConfigurationTranslationFailedEventReason events, +// one per a translation failure causing object. +func (c *KongClient) recordTranslationFailureWarningEvents(translationFailures []parser.TranslationFailure) { + for _, failure := range translationFailures { + for _, obj := range failure.CausingObjects() { + c.eventRecorder.Event(obj, corev1.EventTypeWarning, KongConfigurationTranslationFailedEventReason, failure.Reason()) + } + } +} diff --git a/internal/manager/consts.go b/internal/manager/consts.go index c77e89f5a3..4bc6409b1a 100644 --- a/internal/manager/consts.go +++ b/internal/manager/consts.go @@ -19,3 +19,6 @@ const MetricsPort = 10255 // DiagnosticsPort is the default port of the manager's diagnostics service listens on. const DiagnosticsPort = 10256 + +// KongClientEventRecorderComponentName is a KongClient component name used to identify the events recording component. +const KongClientEventRecorderComponentName = "kong-client" diff --git a/internal/manager/run.go b/internal/manager/run.go index 0e4d6da1c8..49b60227f9 100644 --- a/internal/manager/run.go +++ b/internal/manager/run.go @@ -151,7 +151,18 @@ func RunWithLogger(ctx context.Context, c *Config, diagnostic util.ConfigDumpDia 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) + + eventRecorder := mgr.GetEventRecorderFor(KongClientEventRecorderComponentName) + dataplaneClient, err := dataplane.NewKongClient( + deprecatedLogger, + timeoutDuration, + c.IngressClassName, + c.EnableReverseSync, + c.SkipCACertificates, + diagnostic, + kongConfig, + eventRecorder, + ) if err != nil { return fmt.Errorf("failed to initialize kong data-plane client: %w", err) } @@ -200,7 +211,7 @@ func RunWithLogger(ctx context.Context, c *Config, diagnostic util.ConfigDumpDia // BUG: kubebuilder (at the time of writing - 3.0.0-rc.1) does not allow this tag anywhere else than main.go // See https://github.com/kubernetes-sigs/kubebuilder/issues/932 - //+kubebuilder:scaffold:builder + // +kubebuilder:scaffold:builder setupLog.Info("Starting health check servers") if err := mgr.AddHealthzCheck("health", healthz.Ping); err != nil { diff --git a/test/integration/translation_failures_test.go b/test/integration/translation_failures_test.go new file mode 100644 index 0000000000..c6998422f0 --- /dev/null +++ b/test/integration/translation_failures_test.go @@ -0,0 +1,130 @@ +//go:build integration_tests +// +build integration_tests + +package integration + +import ( + "fmt" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + corev1 "k8s.io/api/core/v1" + 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/annotations" + "github.com/kong/kubernetes-ingress-controller/v2/internal/dataplane" + kongv1 "github.com/kong/kubernetes-ingress-controller/v2/pkg/apis/configuration/v1" + "github.com/kong/kubernetes-ingress-controller/v2/pkg/clientset" +) + +// TestTranslationFailures ensures that proper warning Kubernetes events are recorded in case of translation failures +// encountered. +func TestTranslationFailures(t *testing.T) { + testCases := []struct { + name string + // translationFailureTrigger should create objects that trigger translation failure and return the objects + // that we expect translation failure warning events to be created for. + translationFailureTrigger func(t *testing.T, ns string) []client.Object + }{ + { + name: "invalid CA secret", + translationFailureTrigger: func(t *testing.T, ns string) []client.Object { + createdSecret, err := env.Cluster().Client().CoreV1().Secrets(ns).Create(ctx, invalidCASecret(ns), metav1.CreateOptions{}) + require.NoError(t, err) + + return []client.Object{createdSecret} + }, + }, + { + name: "invalid CA secret referred by a plugin", + translationFailureTrigger: func(t *testing.T, ns string) []client.Object { + createdSecret, err := env.Cluster().Client().CoreV1().Secrets(ns).Create(ctx, invalidCASecret(ns), metav1.CreateOptions{}) + require.NoError(t, err) + + c, err := clientset.NewForConfig(env.Cluster().Config()) + require.NoError(t, err) + createdPlugin, err := c.ConfigurationV1().KongPlugins(ns).Create(ctx, pluginUsingInvalidCACert(ns), metav1.CreateOptions{}) + require.NoError(t, err) + + // expect events for both: a faulty secret and a plugin referring it + return []client.Object{createdSecret, createdPlugin} + }, + }, + } + + for _, tt := range testCases { + tt := tt + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + ns, cleaner := setup(t) + defer func() { assert.NoError(t, cleaner.Cleanup(ctx)) }() + + expectedCausingObjects := tt.translationFailureTrigger(t, ns.GetName()) + + require.Eventually(t, func() bool { + eventsForAllObjectsFound := true + + for _, expectedCausingObject := range expectedCausingObjects { + events, err := env.Cluster().Client().CoreV1().Events(ns.GetName()).List(ctx, metav1.ListOptions{ + FieldSelector: fmt.Sprintf( + "reason=%s,type=%s,involvedObject.name=%s", + dataplane.KongConfigurationTranslationFailedEventReason, + corev1.EventTypeWarning, + expectedCausingObject.GetName(), + ), + }) + if err != nil { + t.Logf("failed to list events: %s", err) + eventsForAllObjectsFound = false + } + + if len(events.Items) == 0 { + t.Logf("waiting for events related to '%s' to be created", expectedCausingObject.GetName()) + eventsForAllObjectsFound = false + } + } + + return eventsForAllObjectsFound + }, time.Minute*5, time.Second) + }) + } +} + +const invalidCASecretID = "8214a145-a328-4c56-ab72-2973a56d4eae" //nolint:gosec + +func invalidCASecret(ns string) *corev1.Secret { + return &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + GenerateName: "ca-secret-", + Namespace: ns, + Labels: map[string]string{ + "konghq.com/ca-cert": "true", + }, + Annotations: map[string]string{ + annotations.IngressClassKey: ingressClass, + }, + }, + Data: map[string][]byte{ + "id": []byte(invalidCASecretID), + // missing cert key + }, + } +} + +func pluginUsingInvalidCACert(ns string) *kongv1.KongPlugin { + return &kongv1.KongPlugin{ + ObjectMeta: metav1.ObjectMeta{ + GenerateName: "kong-plugin-", + Namespace: ns, + Annotations: map[string]string{ + annotations.IngressClassKey: ingressClass, + }, + }, + Config: v1.JSON{Raw: []byte(fmt.Sprintf(`{"ca_certificates": ["%s"]}`, invalidCASecretID))}, + PluginName: "mtls-auth", + } +}