diff --git a/CHANGELOG.md b/CHANGELOG.md index 9fd2b61f62..2e1b885b02 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/go.mod b/go.mod index 2c336b70b3..e4f6546d95 100644 --- a/go.mod +++ b/go.mod @@ -29,7 +29,7 @@ require ( github.com/google/go-cmp v0.6.0 github.com/google/uuid v1.6.0 github.com/jpillora/backoff v1.0.0 - github.com/kong/go-database-reconciler v1.9.0 + github.com/kong/go-database-reconciler v1.10.0 github.com/kong/go-kong v0.54.0 github.com/kong/kubernetes-telemetry v0.1.3 github.com/kong/kubernetes-testing-framework v0.46.0 @@ -202,8 +202,8 @@ require ( golang.org/x/net v0.22.0 // indirect golang.org/x/oauth2 v0.18.0 // indirect golang.org/x/sync v0.7.0 - golang.org/x/sys v0.18.0 // indirect - golang.org/x/term v0.18.0 // indirect + golang.org/x/sys v0.19.0 // indirect + golang.org/x/term v0.19.0 // indirect golang.org/x/text v0.14.0 // indirect golang.org/x/time v0.5.0 // indirect golang.org/x/tools v0.17.0 // indirect diff --git a/go.sum b/go.sum index d7e6a7b6af..88e1a64f83 100644 --- a/go.sum +++ b/go.sum @@ -249,8 +249,8 @@ github.com/kisielk/errcheck v1.5.0/go.mod h1:pFxgyoBC7bSaBwPgfKdkLd5X25qrDl4LWUI github.com/kisielk/gotool v1.0.0/go.mod h1:XhKaO+MFFWcvkIS/tQcRk01m1F5IRFswLeQ+oQHNcck= github.com/klauspost/compress v1.17.0 h1:Rnbp4K9EjcDuVuHtd0dgA4qNuv9yKDYKK1ulpJwgrqM= github.com/klauspost/compress v1.17.0/go.mod h1:ntbaceVETuRiXiv4DpjP66DpAtAGkEQskQzEyD//IeE= -github.com/kong/go-database-reconciler v1.9.0 h1:JWAQlT7lqmN+JrutdkN5yj+u9QwR6P7jNGz7cFqdg4Y= -github.com/kong/go-database-reconciler v1.9.0/go.mod h1:QuHNMiwxuoflH+IkESEZpxO+6KsYXQ4QJ+Rq98wRYCM= +github.com/kong/go-database-reconciler v1.10.0 h1:502Nn7CTsUNZyClL5bjrhkFrendVrPxDkP4ZGlBfsdg= +github.com/kong/go-database-reconciler v1.10.0/go.mod h1:88/u23NIhkQr7SeTP1p4nLcnWCm8AMCwCF1f7Telw+w= github.com/kong/go-kong v0.54.0 h1:HZkZJRREJs/azkgJxMWr2jANQQfg8xXsAiFKu+d1nrs= github.com/kong/go-kong v0.54.0/go.mod h1:51rSSjgSZKukXgn5nNYbJUx0Et/islqR+LTltwCyjG8= github.com/kong/kubernetes-telemetry v0.1.3 h1:Hz2tkHGIIUqbn1x46QRDmmNjbEtJyxyOvHSPne3uPto= @@ -557,14 +557,15 @@ golang.org/x/sys v0.2.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.6.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.8.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.11.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= -golang.org/x/sys v0.18.0 h1:DBdB3niSjOA/O0blCZBqDefyWNYveAYMNF1Wum0DYQ4= golang.org/x/sys v0.18.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= +golang.org/x/sys v0.19.0 h1:q5f1RH2jigJ1MoAWp2KTp3gm5zAGFUTarQZ5U386+4o= +golang.org/x/sys v0.19.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo= golang.org/x/term v0.0.0-20210927222741-03fcf44c2211/go.mod h1:jbD1KX2456YbFQfuXm/mYQcufACuNUgVhRMnK/tPxf8= golang.org/x/term v0.0.0-20220526004731-065cf7ba2467/go.mod h1:jbD1KX2456YbFQfuXm/mYQcufACuNUgVhRMnK/tPxf8= golang.org/x/term v0.2.0/go.mod h1:TVmDHMZPmdnySmBfhjOoOdhjzdE1h4u1VwSiw2l1Nuc= -golang.org/x/term v0.18.0 h1:FcHjZXDMxI8mM3nwhX9HlKop4C0YQvCVCdwYl2wOtE8= -golang.org/x/term v0.18.0/go.mod h1:ILwASektA3OnRv7amZ1xhE/KTR+u50pbXfZ03+6Nx58= +golang.org/x/term v0.19.0 h1:+ThwsDv+tYfnJFhF4L8jITxu1tdTWRTZpdsWgEgjL6Q= +golang.org/x/term v0.19.0/go.mod h1:2CuTdWZ7KHSQwUzKva0cbMg6q2DMI3Mmxp+gKJbskEk= golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= golang.org/x/text v0.3.3/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ= golang.org/x/text v0.3.7/go.mod h1:u+2+/6zg+i71rQMx5EYifcz6MCKuco9NR6JIITiCfzQ= diff --git a/internal/dataplane/sendconfig/dbmode.go b/internal/dataplane/sendconfig/dbmode.go index 7829f81155..9ccce2e506 100644 --- a/internal/dataplane/sendconfig/dbmode.go +++ b/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,16 +17,20 @@ 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( @@ -31,12 +38,16 @@ func NewUpdateStrategyDBMode( 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,111 @@ 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() + s.resourceErrorLock.Lock() + defer s.resourceErrorLock.Unlock() 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() + 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, "failed updating gateway entity", "action", event.Action, "kind", event.Entity.Kind, "name", event.Entity.Name) + parsed, err := resourceErrorFromEntityAction(event) + if err != nil { + s.logger.Error(err, "could not parse entity update error") + } else { + s.resourceErrors = append(s.resourceErrors, parsed) + } + } + case <-ctx.Done(): + s.resourceErrorLock.Unlock() + return + } + } +} + +func resourceErrorFromEntityAction(event diff.EntityAction) (ResourceError, 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 ResourceError{}, 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 ResourceError{}, 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 ResourceError{}, 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) + } + + // This omits ID, which should be available but requires similar reflect gymnastics as Tags, and probably isn't worth + // it. + raw := 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), + }, } - return nil, nil, nil, nil + return parseRawResourceError(raw) } func (s UpdateStrategyDBMode) MetricsProtocol() metrics.Protocol { diff --git a/internal/dataplane/sendconfig/strategy.go b/internal/dataplane/sendconfig/strategy.go index cedf1081b4..a460be1949 100644 --- a/internal/dataplane/sendconfig/strategy.go +++ b/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, ) } diff --git a/internal/manager/config.go b/internal/manager/config.go index 4405778a84..486269d556 100644 --- a/internal/manager/config.go +++ b/internal/manager/config.go @@ -83,6 +83,7 @@ type Config struct { IngressClassName string LeaderElectionNamespace string LeaderElectionID string + LeaderElectionForce string Concurrency int FilterTags []string WatchNamespaces []string @@ -216,6 +217,8 @@ func (c *Config) FlagSet() *pflag.FlagSet { flagSet.StringVar(&c.IngressClassName, "ingress-class", annotations.DefaultIngressClass, `Name of the ingress class to route through this controller.`) flagSet.StringVar(&c.LeaderElectionID, "election-id", "5b374a9e.konghq.com", `Election id to use for status update.`) flagSet.StringVar(&c.LeaderElectionNamespace, "election-namespace", "", `Leader election namespace to use when running outside a cluster.`) + flagSet.StringVar(&c.LeaderElectionForce, "force-leader-election", "", `Set to "enabled" or "disabled" to force a leader election behavior. Behavior is normally determined automatically from other settings.`) + _ = flagSet.MarkHidden("force-leader-election") flagSet.StringSliceVar(&c.FilterTags, "kong-admin-filter-tag", []string{"managed-by-ingress-controller"}, "Tag(s) in comma-separated format (or specify this flag multiple times). They are used to manage and filter entities in Kong. "+ "This setting will be silently ignored if the Kong instance has no tags support.") diff --git a/internal/manager/setup.go b/internal/manager/setup.go index 0c0b74f8a1..1a4c671099 100644 --- a/internal/manager/setup.go +++ b/internal/manager/setup.go @@ -120,7 +120,20 @@ func setupManagerOptions(ctx context.Context, logger logr.Logger, c *Config, dbm return managerOpts, nil } +const ( + LeaderElectionEnabled = "enabled" + LeaderElectionDisabled = "disabled" +) + func leaderElectionEnabled(logger logr.Logger, c *Config, dbmode dpconf.DBMode) bool { + if c.LeaderElectionForce == LeaderElectionEnabled { + logger.Info("leader election forcibly enabled") + return true + } + if c.LeaderElectionForce == LeaderElectionDisabled { + logger.Info("leader election forcibly disabled") + return false + } if c.Konnect.ConfigSynchronizationEnabled { logger.Info("Konnect config synchronisation enabled, enabling leader election") return true diff --git a/test/envtest/configerrorevent_envtest_test.go b/test/envtest/configerrorevent_envtest_test.go index 815885e944..45a16af16b 100644 --- a/test/envtest/configerrorevent_envtest_test.go +++ b/test/envtest/configerrorevent_envtest_test.go @@ -5,6 +5,8 @@ package envtest import ( "bytes" "context" + "errors" + "fmt" "regexp" "testing" "text/template" @@ -12,18 +14,23 @@ import ( "github.com/kong/kubernetes-testing-framework/pkg/utils/kubernetes/generators" "github.com/samber/lo" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" corev1 "k8s.io/api/core/v1" netv1 "k8s.io/api/networking/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/kubernetes/scheme" "sigs.k8s.io/controller-runtime/pkg/client" + "github.com/kong/kubernetes-ingress-controller/v3/internal/annotations" "github.com/kong/kubernetes-ingress-controller/v3/internal/dataplane" + kongv1 "github.com/kong/kubernetes-ingress-controller/v3/pkg/apis/configuration/v1" "github.com/kong/kubernetes-ingress-controller/v3/test" "github.com/kong/kubernetes-ingress-controller/v3/test/mocks" ) -func TestConfigErrorEventGeneration(t *testing.T) { +func TestConfigErrorEventGenerationInMemoryMode(t *testing.T) { // Can't be run in parallel because we're using t.Setenv() below which doesn't allow it. const ( @@ -242,3 +249,109 @@ func formatErrBody(t *testing.T, namespace string, ingress *netv1.Ingress, servi return b.Bytes() } + +func TestConfigErrorEventGenerationDBMode(t *testing.T) { + // Can't be run in parallel because we're using t.Setenv() below which doesn't allow it. + + const ( + waitTime = time.Minute + tickTime = 100 * time.Millisecond + ) + + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + scheme := Scheme(t, WithKong) + restConfig := Setup(t, scheme) + ctrlClientGlobal := NewControllerClient(t, scheme, restConfig) + ns := CreateNamespace(ctx, t, ctrlClientGlobal) + ctrlClient := client.NewNamespacedClient(ctrlClientGlobal, ns.Name) + + ingressClassName := "kongenvtest" + deployIngressClass(ctx, t, ingressClassName, ctrlClient) + + const podName = "kong-ingress-controller-tyjh1" + t.Setenv("POD_NAMESPACE", ns.Name) + t.Setenv("POD_NAME", podName) + + t.Logf("creating a static consumer in %s namespace which will be used to test global validation", ns.Name) + consumer := &kongv1.KongConsumer{ + ObjectMeta: metav1.ObjectMeta{ + Name: "donenbai", + Annotations: map[string]string{ + annotations.IngressClassKey: ingressClassName, + }, + }, + Username: "donenbai", + } + require.NoError(t, ctrlClient.Create(ctx, consumer)) + t.Cleanup(func() { + if err := ctrlClient.Delete(ctx, consumer); err != nil && !apierrors.IsNotFound(err) && !errors.Is(err, context.Canceled) { + assert.NoError(t, err) + } + }) + + RunManager(ctx, t, restConfig, + AdminAPIOptFns( + // TODO IDK where we're getting the version from normally but it shouldn't really matter for this. + mocks.WithRoot(formatDBRootResponse("999.999.999")), + ), + WithPublishService(ns.Name), + WithIngressClass(ingressClassName), + WithProxySyncSeconds(0.1), + ) + + t.Log("checking kongconsumer event creation") + require.Eventually(t, func() bool { + var events corev1.EventList + if err := ctrlClient.List(ctx, &events, &client.ListOptions{Namespace: ns.Name}); err != nil { + t.Logf("error listing events: %v", err) + return false + } + t.Logf("got %d events", len(events.Items)) + + matches := make([]bool, 1) + matches[0] = lo.ContainsBy(events.Items, func(e corev1.Event) bool { + return e.Reason == dataplane.KongConfigurationApplyFailedEventReason && + e.InvolvedObject.Kind == "KongConsumer" && + e.InvolvedObject.Name == consumer.Name && + e.Message == "invalid : HTTP status 400 (message: \"2 schema violations (at least one of these fields must be non-empty: 'custom_id', 'username'; fake: unknown field)\")" + }) + if lo.Count(matches, true) != 1 { + t.Logf("not all events matched: %+v", matches) + return false + } + return true + }, waitTime, tickTime) + + t.Log("push failure events recorded successfully") +} + +func formatDBRootResponse(version string) []byte { + const defaultDBLessRootResponse = `{ + "version": "%s", + "configuration": { + "database": "postgres", + "router_flavor": "traditional", + "role": "traditional", + "proxy_listeners": [ + { + "ipv6only=on": false, + "ipv6only=off": false, + "ssl": false, + "so_keepalive=off": false, + "listener": "0.0.0.0:8000", + "bind": false, + "port": 8000, + "deferred": false, + "so_keepalive=on": false, + "http2": false, + "proxy_protocol": false, + "ip": "0.0.0.0", + "reuseport": false + } + ] + } + }` + return []byte(fmt.Sprintf(defaultDBLessRootResponse, version)) +} diff --git a/test/envtest/run.go b/test/envtest/run.go index 76ea933b31..9323675e78 100644 --- a/test/envtest/run.go +++ b/test/envtest/run.go @@ -78,6 +78,9 @@ func ConfigForEnvConfig(t *testing.T, envcfg *rest.Config, opts ...mocks.AdminAP cfg.GatewayAPIHTTPRouteController = false cfg.GatewayAPIReferenceGrantController = false + // Disable leader election, which doesn't work outside the cluster and is irrelevant for single-instance tests. + cfg.LeaderElectionForce = manager.LeaderElectionDisabled + return cfg } diff --git a/test/integration/suite_test.go b/test/integration/suite_test.go index 883b31bc44..3774299e6f 100644 --- a/test/integration/suite_test.go +++ b/test/integration/suite_test.go @@ -20,6 +20,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" gatewayclient "sigs.k8s.io/gateway-api/pkg/client/clientset/versioned" + "github.com/kong/kubernetes-ingress-controller/v3/internal/manager" testutils "github.com/kong/kubernetes-ingress-controller/v3/internal/util/test" "github.com/kong/kubernetes-ingress-controller/v3/test" "github.com/kong/kubernetes-ingress-controller/v3/test/consts" @@ -184,6 +185,11 @@ func TestMain(m *testing.M) { "--anonymous-reports=false", fmt.Sprintf("--feature-gates=%s", featureGates), fmt.Sprintf("--election-namespace=%s", kongAddon.Namespace()), + // Leader election is irrelevant for single-instance tests. We should effectively always be the leader. However, + // controller-runtime operates an internal leadership deadline and will abort if it cannot update leadership + // within a certain number of seconds. Pausing certain segments manager in a debugger can exceed this deadline, + // so elections are disabled in integration tests for convenience. + fmt.Sprintf("force-leader-election=%s", manager.LeaderElectionDisabled), } allControllerArgs := append(standardControllerArgs, extraControllerArgs...) cancel, err := testutils.DeployControllerManagerForCluster(ctx, logger, env.Cluster(), kongAddon, allControllerArgs) diff --git a/test/kongintegration/translator_golden_tests_outputs_test.go b/test/kongintegration/translator_golden_tests_outputs_test.go index 5af8afa3ef..d56064279f 100644 --- a/test/kongintegration/translator_golden_tests_outputs_test.go +++ b/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) { diff --git a/test/mocks/admin_api_handler.go b/test/mocks/admin_api_handler.go index b897db48aa..9b1f8ffb7b 100644 --- a/test/mocks/admin_api_handler.go +++ b/test/mocks/admin_api_handler.go @@ -10,6 +10,18 @@ import ( "github.com/kong/kubernetes-ingress-controller/v3/internal/versions" ) +const mockConsumerError = `{ + "code": 2, + "fields": { + "@entity": [ + "at least one of these fields must be non-empty: 'custom_id', 'username'" + ], + "fake": "unknown field" + }, + "message": "2 schema violations (at least one of these fields must be non-empty: 'custom_id', 'username'; fake: unknown field)", + "name": "schema violation" +}` + // AdminAPIHandler is a mock implementation of the Admin API. It only implements the endpoints that are // required for the tests. type AdminAPIHandler struct { @@ -40,6 +52,9 @@ type AdminAPIHandler struct { // configPostErrorBody contains the error body which will be returned when // responding to a `POST /config` request. configPostErrorBody []byte + + // rootResponse is the response body served by the admin API root "GET /" endpoint. + rootResponse []byte } type AdminAPIHandlerOpt func(h *AdminAPIHandler) @@ -79,6 +94,12 @@ func WithConfigPostError(errorbody []byte) AdminAPIHandlerOpt { } } +func WithRoot(response []byte) AdminAPIHandlerOpt { + return func(h *AdminAPIHandler) { + h.rootResponse = response + } +} + func NewAdminAPIHandler(t *testing.T, opts ...AdminAPIHandlerOpt) *AdminAPIHandler { h := &AdminAPIHandler{ version: versions.KICv3VersionCutoff.String(), @@ -94,7 +115,11 @@ func NewAdminAPIHandler(t *testing.T, opts ...AdminAPIHandlerOpt) *AdminAPIHandl mux.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) { if r.Method == http.MethodGet { - _, _ = w.Write(formatDefaultDBLessRootResponse(h.version)) + if h.rootResponse != nil { + _, _ = w.Write(h.rootResponse) + } else { + _, _ = w.Write(formatDefaultDBLessRootResponse(h.version)) + } return } @@ -165,6 +190,53 @@ func NewAdminAPIHandler(t *testing.T, opts ...AdminAPIHandlerOpt) *AdminAPIHandl t.Errorf("unexpected request: %s %s", r.Method, r.URL) } }) + // These endpoints are only used for the DB mode test. They always return empty responses for GETs and a fake error + // for POSTs and PUTs, to test error handling. + mux.HandleFunc("/consumers/{$}", func(w http.ResponseWriter, r *http.Request) { + switch r.Method { + case http.MethodGet: + w.WriteHeader(http.StatusOK) + _, _ = w.Write([]byte(`{ "data": [], "next": null }`)) + + case http.MethodPost: + w.WriteHeader(http.StatusBadRequest) + _, _ = w.Write([]byte(mockConsumerError)) + + case http.MethodPut: + w.WriteHeader(http.StatusBadRequest) + _, _ = w.Write([]byte(mockConsumerError)) + + default: + t.Errorf("unexpected request: %s %s", r.Method, r.URL) + } + }) + mux.HandleFunc("/consumers/", func(w http.ResponseWriter, r *http.Request) { + switch r.Method { + case http.MethodGet: + // This should technically be the below 404. The mux termination for handling "/consumers/" and + // "/consumers/" isn't behaving as expected and always returns this less specific endpoint. + // Returning the expected 404 here breaks GDR because it can't find the list of existing consumers. + // Returning the 200 list response even for more specific queries is apparently fine for the purposes of the + // test, so we do so as a hack to get to the endpoint we care about, "POST/PUT /consumers/". + // + //w.WriteHeader(http.StatusNotFound) + //_, _ = w.Write([]byte(`{ "message": "Not found" }`)) + // + w.WriteHeader(http.StatusOK) + _, _ = w.Write([]byte(`{ "data": [], "next": null }`)) + + case http.MethodPost: + w.WriteHeader(http.StatusBadRequest) + _, _ = w.Write([]byte(mockConsumerError)) + + case http.MethodPut: + w.WriteHeader(http.StatusBadRequest) + _, _ = w.Write([]byte(mockConsumerError)) + + default: + t.Errorf("unexpected request: %s %s", r.Method, r.URL) + } + }) h.mux = mux return h }