Merge branch 'main' into backendtlspolicy-customize-services

CHANGELOG.md

@@ -128,6 +128,11 @@ Adding a new version? You'll need three changes:
 - Log `Object requested backendRef to target, but it does not exist, skipping...`
   as `DEBUG` instead of `ERROR`, enhance `HTTPRoute` status with detailed message.
   [#6746](https://github.com/Kong/kubernetes-ingress-controller/pull/6746)
+- Logs related to misconfiguration of objects like `object failed to apply...`
+  and `resource processing failed` are logged as `DEBUG` instead of `ERROR`,
+  any needed information is reported in the status of the affected object or
+  with Kubernetes events.
+  [#6790](https://github.com/Kong/kubernetes-ingress-controller/pull/6790)
 - From now on, upstreams produced by KIC from `Service`s that are configured as
   upstream services (either by `ingress.kubernetes.io/service-upstream` annotation
   or through `IngressClassNamespacedParameters`'s `serviceUpstream` field), will use

internal/dataplane/configfetcher/kongrawstate_test.go

@@ -361,7 +361,7 @@ func extractNotEmptyFieldNames(s kongstate.KongState) []string {
 	for i := 0; i < typ.NumField(); i++ {
 		f := typ.Field(i)
 		v := reflect.ValueOf(s).Field(i)
-		if !f.Anonymous && f.IsExported() && !v.IsZero() {
+		if !f.Anonymous && f.IsExported() && v.IsValid() && !v.IsZero() {
 			fields = append(fields, f.Name)
 		}
 	}

internal/dataplane/failures/failures.go

@@ -6,6 +6,8 @@ import (
 
 	"github.com/go-logr/logr"
 	"sigs.k8s.io/controller-runtime/pkg/client"
+
+	"github.com/kong/kubernetes-ingress-controller/v3/internal/logging"
 )
 
 const (
@@ -84,7 +86,8 @@ func (c *ResourceFailuresCollector) PushResourceFailure(reason string, causingOb
 // logResourceFailure logs an error with a resource processing failure message for each causing object.
 func (c *ResourceFailuresCollector) logResourceFailure(reason string, causingObjects ...client.Object) {
 	for _, obj := range causingObjects {
-		c.logger.Error(fmt.Errorf("resource processing failed"), reason,
+		c.logger.V(logging.DebugLevel).Info("resource processing failed",
+			"reason", reason,
 			"name", obj.GetName(),
 			"namespace", obj.GetNamespace(),
 			"GVK", obj.GetObjectKind().GroupVersionKind().String())

internal/dataplane/failures/failures_test.go

@@ -71,7 +71,7 @@ func TestResourceFailuresCollector(t *testing.T) {
 	})
 
 	t.Run("pushes, logs and pops resource failures", func(t *testing.T) {
-		core, logs := observer.New(zap.InfoLevel)
+		core, logs := observer.New(zap.DebugLevel)
 		logger := zapr.NewLogger(zap.New(core))
 
 		collector := NewResourceFailuresCollector(logger)
@@ -80,8 +80,8 @@ func TestResourceFailuresCollector(t *testing.T) {
 		collector.PushResourceFailure(someValidResourceFailureReason, someResourceFailureCausingObjects()...)
 
 		numberOfCausingObjects := len(someResourceFailureCausingObjects())
-		require.Equal(t, logs.Len(), numberOfCausingObjects*2, "expecting one log entry per causing object")
-		assertErrorLogs(t, logs)
+		require.Equal(t, numberOfCausingObjects*2, logs.Len(), "expecting one log entry per causing object")
+		assertDebugLogs(t, logs)
 
 		collectedErrors := collector.PopResourceFailures()
 		require.Len(t, collectedErrors, 2)
@@ -104,9 +104,9 @@ func TestResourceFailuresCollector(t *testing.T) {
 	})
 }
 
-func assertErrorLogs(t *testing.T, logs *observer.ObservedLogs) {
+func assertDebugLogs(t *testing.T, logs *observer.ObservedLogs) {
 	for i := range logs.All() {
-		assert.Equalf(t, zapcore.ErrorLevel, logs.All()[i].Entry.Level, "%d-nth log entry expected to have ErrorLevel", i)
+		assert.Equalf(t, zapcore.DebugLevel, logs.All()[i].Entry.Level, "%d-nth log entry expected to have DebugLevel", i)
 	}
 }
 

internal/dataplane/kong_client.go

@@ -813,16 +813,19 @@ func (c *KongClient) sendToClient(
 			updateErr          sendconfig.UpdateError
 			responseParsingErr sendconfig.ResponseParsingError
 		)
-		if errors.As(err, &updateErr) {
+
+		switch {
+		case errors.As(err, &updateErr):
 			reason := KongConfigurationApplyFailedEventReason
 			if isFallback {
 				reason = FallbackKongConfigurationApplyFailedEventReason
 			}
 			c.recordResourceFailureEvents(updateErr.ResourceFailures(), reason)
-		}
-		if errors.As(err, &responseParsingErr) {
+			rawResponseBody = updateErr.RawResponseBody()
+		case errors.As(err, &responseParsingErr):
 			rawResponseBody = responseParsingErr.ResponseBody()
 		}
+
 		sendDiagnostic(diagnostics.DumpMeta{Failed: true, Hash: string(newConfigSHA)}, rawResponseBody)
 
 		if err := ctx.Err(); err != nil {
@@ -967,13 +970,12 @@ func (c *KongClient) recordResourceFailureEvents(resourceFailures []failures.Res
 	for _, failure := range resourceFailures {
 		for _, obj := range failure.CausingObjects() {
 			gvk := obj.GetObjectKind().GroupVersionKind()
-			c.logger.Error(
-				errors.New("object failed to apply"),
-				"recording a Warning event for object",
+			c.logger.V(logging.DebugLevel).Info(
+				"object failed to apply - recording a Warning event for object",
 				"name", obj.GetName(),
 				"namespace", obj.GetNamespace(),
 				"kind", gvk.Kind,
-				"apiVersion", gvk.Group+"/"+gvk.Version,
+				"apiVersion", gvk.GroupVersion().String(),
 				"reason", reason,
 				"message", failure.Message(),
 			)

internal/dataplane/kongstate/kongstate_test.go

@@ -206,7 +206,7 @@ func extractNotEmptyFieldNames(s KongState) []string {
 	for i := 0; i < typ.NumField(); i++ {
 		f := typ.Field(i)
 		v := reflect.ValueOf(s).Field(i)
-		if !f.Anonymous && f.IsExported() && !v.IsZero() {
+		if !f.Anonymous && f.IsExported() && v.IsValid() && !v.IsZero() {
 			fields = append(fields, f.Name)
 		}
 	}

internal/dataplane/sendconfig/dbmode.go

@@ -162,7 +162,7 @@ func resourceErrorFromEntityAction(event diff.EntityAction) (ResourceError, erro
 			event.Entity.Kind, event.Entity.Name, reflected.Kind())
 	}
 	tagsValue := reflected.FieldByName("Tags")
-	if tagsValue.IsZero() {
+	if !tagsValue.IsValid() || tagsValue.IsZero() {
 		return ResourceError{}, fmt.Errorf("entity %s/%s of type %s lacks 'Tags' field",
 			event.Entity.Kind, event.Entity.Name, reflect.TypeOf(subj))
 	}

internal/dataplane/translator/ingressrules_test.go

@@ -366,11 +366,11 @@ func TestGetK8sServicesForBackends(t *testing.T) {
 
 func TestDoK8sServicesMatchAnnotations(t *testing.T) {
 	for _, tt := range []struct {
-		name               string
-		services           []*corev1.Service
-		annotations        map[string]string
-		expected           bool
-		expectedLogEntries []string
+		name                     string
+		services                 []*corev1.Service
+		annotations              map[string]string
+		expected                 bool
+		expectedLogReasonEntries []string
 	}{
 		{
 			name:        "if no services are provided, then there's no validation failure",
@@ -495,7 +495,7 @@ func TestDoK8sServicesMatchAnnotations(t *testing.T) {
 				"konghq.com/baz": "foo",
 			},
 			expected: false,
-			expectedLogEntries: []string{
+			expectedLogReasonEntries: []string{
 				"Service has inconsistent konghq.com/foo annotation and is used in multi-Service backend",
 				"Service has inconsistent konghq.com/foo annotation and is used in multi-Service backend",
 				"Service has inconsistent konghq.com/foo annotation and is used in multi-Service backend",
@@ -539,22 +539,25 @@ func TestDoK8sServicesMatchAnnotations(t *testing.T) {
 				"konghq.com/foo": "bar",
 			},
 			expected: false,
-			expectedLogEntries: []string{
+			expectedLogReasonEntries: []string{
 				"Service has inconsistent konghq.com/foo annotation and is used in multi-Service backend",
 				"Service has inconsistent konghq.com/foo annotation and is used in multi-Service backend",
 				"Service has inconsistent konghq.com/foo annotation and is used in multi-Service backend",
 			},
 		},
 	} {
 		t.Run(tt.name, func(t *testing.T) {
-			core, logs := observer.New(zap.InfoLevel)
+			core, logs := observer.New(zap.DebugLevel)
 			logger := zapr.NewLogger(zap.New(core))
-
 			failuresCollector := failures.NewResourceFailuresCollector(logger)
 			assert.Equal(t, tt.expected, collectInconsistentAnnotations(tt.services, tt.annotations, failuresCollector, ""))
-			assert.Len(t, failuresCollector.PopResourceFailures(), len(tt.expectedLogEntries), "expecting as many translation failures as log entries")
-			for i := range tt.expectedLogEntries {
-				assert.Contains(t, logs.All()[i].Entry.Message, tt.expectedLogEntries[i])
+			assert.Len(t, failuresCollector.PopResourceFailures(), len(tt.expectedLogReasonEntries), "expecting as many translation failures as log entries")
+
+			allLogReasonEntries := lo.Map(logs.All(), func(e observer.LoggedEntry, _ int) string {
+				return e.ContextMap()["reason"].(string)
+			})
+			for i := range tt.expectedLogReasonEntries {
+				assert.Contains(t, allLogReasonEntries[i], tt.expectedLogReasonEntries[i])
 			}
 		})
 	}