feat(sendconfig) parse resource errors in DB mode

Use the GDR result channel to log actions and build resource errors from
failed actions.

chore: update CHANGELOG

CHANGELOG.md

@@ -99,6 +99,8 @@ Adding a new version? You'll need three changes:
   [#5787](https://github.com/Kong/kubernetes-ingress-controller/pull/5787)
 - Add support in `HTTPRoute`s for `URLRewrite`:
   - `FullPathRewrite` [#5855](https://github.com/Kong/kubernetes-ingress-controller/pull/5855)
+- DB mode now supports Event reporting for resources that failed to apply.
+  [#5785](https://github.com/Kong/kubernetes-ingress-controller/pull/5785)
 
 ### Fixed
 

internal/dataplane/sendconfig/dbmode.go

@@ -3,8 +3,11 @@ package sendconfig
 import (
 	"context"
 	"fmt"
+	"reflect"
+	"sync"
 
 	"github.com/blang/semver/v4"
+	"github.com/go-logr/logr"
 	"github.com/kong/go-database-reconciler/pkg/diff"
 	"github.com/kong/go-database-reconciler/pkg/dump"
 	"github.com/kong/go-database-reconciler/pkg/file"
@@ -14,29 +17,37 @@ import (
 
 	"github.com/kong/kubernetes-ingress-controller/v3/internal/dataplane/deckerrors"
 	"github.com/kong/kubernetes-ingress-controller/v3/internal/metrics"
+	"github.com/kong/kubernetes-ingress-controller/v3/internal/util"
 )
 
 // UpdateStrategyDBMode implements the UpdateStrategy interface. It updates Kong's data-plane
 // configuration using decK's syncer.
 type UpdateStrategyDBMode struct {
-	client      *kong.Client
-	dumpConfig  dump.Config
-	version     semver.Version
-	concurrency int
-	isKonnect   bool
+	client            *kong.Client
+	dumpConfig        dump.Config
+	version           semver.Version
+	concurrency       int
+	isKonnect         bool
+	logger            logr.Logger
+	resourceErrors    []ResourceError
+	resourceErrorLock *sync.Mutex
 }
 
 func NewUpdateStrategyDBMode(
 	client *kong.Client,
 	dumpConfig dump.Config,
 	version semver.Version,
 	concurrency int,
+	logger logr.Logger,
 ) UpdateStrategyDBMode {
 	return UpdateStrategyDBMode{
-		client:      client,
-		dumpConfig:  dumpConfig,
-		version:     version,
-		concurrency: concurrency,
+		client:            client,
+		dumpConfig:        dumpConfig,
+		version:           version,
+		concurrency:       concurrency,
+		logger:            logger,
+		resourceErrors:    []ResourceError{},
+		resourceErrorLock: &sync.Mutex{},
 	}
 }
 
@@ -45,8 +56,9 @@ func NewUpdateStrategyDBModeKonnect(
 	dumpConfig dump.Config,
 	version semver.Version,
 	concurrency int,
+	logger logr.Logger,
 ) UpdateStrategyDBMode {
-	s := NewUpdateStrategyDBMode(client, dumpConfig, version, concurrency)
+	s := NewUpdateStrategyDBMode(client, dumpConfig, version, concurrency, logger)
 	s.isKonnect = true
 	return s
 }
@@ -70,23 +82,114 @@ func (s UpdateStrategyDBMode) Update(ctx context.Context, targetContent ContentW
 	}
 
 	syncer, err := diff.NewSyncer(diff.SyncerOpts{
-		CurrentState:    cs,
-		TargetState:     ts,
-		KongClient:      s.client,
-		SilenceWarnings: true,
-		IsKonnect:       s.isKonnect,
-		IncludeLicenses: true,
+		CurrentState:        cs,
+		TargetState:         ts,
+		KongClient:          s.client,
+		SilenceWarnings:     true,
+		IsKonnect:           s.isKonnect,
+		IncludeLicenses:     true,
+		EnableEntityActions: true,
 	})
 	if err != nil {
 		return fmt.Errorf("creating a new syncer for %s: %w", s.client.BaseRootURL(), err), nil, nil, nil
 	}
 
+	ctx, cancel := context.WithCancel(ctx)
+	go s.HandleEvents(ctx, syncer.GetResultChan())
+
 	_, errs, _ := syncer.Solve(ctx, s.concurrency, false, false)
+	cancel()
 	if errs != nil {
-		return deckutils.ErrArray{Errors: errs}, nil, nil, nil
+		return deckutils.ErrArray{Errors: errs}, s.resourceErrors, nil, nil
+	}
+
+	// as of GDR 1.8 we should always get a plain error set in addition to resourceErrors, so returning resourceErrors
+	// here should not be necessary. Return it anyway as a future-proof because why not.
+	return nil, s.resourceErrors, nil, nil
+}
+
+// HandleEvents handles logging and error reporting for individual entity change events generated during a sync by
+// looping over an event channel. It terminates when its context dies.
+func (s UpdateStrategyDBMode) HandleEvents(ctx context.Context, events chan diff.EntityAction) {
+	s.resourceErrorLock.Lock()
+	count := 0
+	for {
+		select {
+		case event := <-events:
+			if event.Error == nil {
+				s.logger.V(util.DebugLevel).Info("updated gateway entity", "action", event.Action, "kind", event.Entity.Kind, "name", event.Entity.Name)
+			} else {
+				s.logger.Error(event.Error, "action", event.Action, "kind", event.Entity.Kind, "name", event.Entity.Name)
+				raw, err := resourceErrorFromEntityAction(event)
+				if err != nil {
+					s.logger.Error(err, "could not parse entity update error")
+				} else {
+					rerror, err := parseRawResourceError(raw)
+					if err != nil {
+						s.logger.Error(err, "could not parse entity update error")
+					} else {
+						s.resourceErrors = append(s.resourceErrors, rerror)
+						count++
+					}
+				}
+			}
+		case <-ctx.Done():
+			s.resourceErrorLock.Unlock()
+			return
+		}
+	}
+}
+
+func resourceErrorFromEntityAction(event diff.EntityAction) (rawResourceError, error) {
+	var subj any
+	// GDR may produce an old only (delete), new only (create), or both (update) in an event. tags should be identical
+	// but we arbitrarily pull from new.
+	if event.Entity.New != nil {
+		subj = event.Entity.New
+	} else {
+		subj = event.Entity.Old
+	}
+	// GDR makes frequent use of "any" for its various entity handlers. It does not use interfaces that would allow us
+	// to guarantee that a particular entity does indeed have tags or similar and retrieve them. We're unlikely to
+	// refactor this any time soon, so in absence of proper interface methods, we pray that the entity probably has tags,
+	// which is a reasonable assumption as anything KIC can manage does. The reflect-fu here is sinister and menacing,
+	// but should spit out tags unless something has gone wrong.
+	reflected := reflect.Indirect(reflect.ValueOf(subj))
+	if reflected.Kind() != reflect.Struct {
+		// We need to fail fast here because FieldByName() will panic on non-Struct Kinds.
+		return rawResourceError{}, fmt.Errorf("entity %s/%s is %s, not Struct",
+			event.Entity.Kind, event.Entity.Name, reflected.Kind())
+	}
+	tagsValue := reflected.FieldByName("Tags")
+	if tagsValue.IsZero() {
+		return rawResourceError{}, fmt.Errorf("entity %s/%s of type %s lacks 'Tags' field",
+			event.Entity.Kind, event.Entity.Name, reflect.TypeOf(subj))
+	}
+	tags, ok := tagsValue.Interface().([]*string)
+	if !ok {
+		return rawResourceError{}, fmt.Errorf("entity %s/%s Tags field is not []*string",
+			event.Entity.Kind, event.Entity.Name)
+	}
+
+	actualTags := []string{}
+	for _, s := range tags {
+		actualTags = append(actualTags, *s)
 	}
 
-	return nil, nil, nil, nil
+	// This omits ID, which should be available but requires similar reflect gymnastics as Tags, and probably isn't worth
+	// it.
+	return rawResourceError{
+		Name: event.Entity.Name,
+		Tags: actualTags,
+		// /config flattened errors have a structured set of field to error reasons, whereas GDR errors are just plain
+		// un-parsed admin API endpoint strings. These will often mention a field within the string, e.g.
+		// schema violation (methods: cannot set 'methods' when 'protocols' is 'grpc' or 'grpcs')
+		// has "methods", but we'd need to do string parsing to extract it, and we may not catch all possible error types.
+		// This lazier approach just dumps the full error string as a single problem, which is probably good enough.
+		Problems: map[string]string{
+			"": fmt.Sprintf("%s", event.Error),
+		},
+	}, nil
 }
 
 func (s UpdateStrategyDBMode) MetricsProtocol() metrics.Protocol {

internal/dataplane/sendconfig/strategy.go

@@ -98,6 +98,7 @@ func (r DefaultUpdateStrategyResolver) resolveUpdateStrategy(client UpdateClient
 			},
 			r.config.Version,
 			r.config.Concurrency,
+			r.logger,
 		)
 	}
 
@@ -111,6 +112,7 @@ func (r DefaultUpdateStrategyResolver) resolveUpdateStrategy(client UpdateClient
 			},
 			r.config.Version,
 			r.config.Concurrency,
+			r.logger,
 		)
 	}
 

test/kongintegration/translator_golden_tests_outputs_test.go

@@ -104,7 +104,7 @@ func TestTranslatorsGoldenTestsOutputs_Konnect(t *testing.T) {
 	updateStrategy := sendconfig.NewUpdateStrategyDBModeKonnect(adminAPIClient.AdminAPIClient(), dump.Config{
 		SkipCACerts:         true,
 		KonnectControlPlane: cpID,
-	}, semver.MustParse("3.5.0"), 10)
+	}, semver.MustParse("3.5.0"), 10, logr.Discard())
 
 	for _, goldenTestOutputPath := range allGoldenTestsOutputsPaths(t) {
 		t.Run(goldenTestOutputPath, func(t *testing.T) {