Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Konnect synchronizer use client from clients provider #6905

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions internal/dataplane/kong_client_golden_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -291,8 +291,8 @@ func runKongClientGoldenTest(t *testing.T, tc kongClientGoldenTestCase) {
ExpressionRoutes: tc.featureFlags.ExpressionRoutes,
FallbackConfiguration: len(objectsToBeConsideredBroken) > 0,
}
clientsProvider := &mockGatewayClientsProvider{
gatewayClients: []*adminapi.Client{adminAPIClient},
clientsProvider := &mocks.MockGatewayClientsProvider{
GatewayClientList: []*adminapi.Client{adminAPIClient},
}
updateStrategyResolver := sendconfig.NewDefaultUpdateStrategyResolver(cfg, logger)
lastValidConfigFetcher := configfetcher.NewDefaultKongLastGoodConfigFetcher(tc.featureFlags.FillIDs, "default")
Expand Down
117 changes: 46 additions & 71 deletions internal/dataplane/kong_client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -143,31 +143,6 @@ var (
}
)

// mockGatewayClientsProvider is a mock implementation of dataplane.AdminAPIClientsProvider.
type mockGatewayClientsProvider struct {
gatewayClients []*adminapi.Client
konnectClient *adminapi.KonnectClient
dbMode dpconf.DBMode
}

func (p *mockGatewayClientsProvider) KonnectClient() *adminapi.KonnectClient {
return p.konnectClient
}

func (p *mockGatewayClientsProvider) GatewayClients() []*adminapi.Client {
return p.gatewayClients
}

func (p *mockGatewayClientsProvider) GatewayClientsToConfigure() []*adminapi.Client {
if p.dbMode.IsDBLessMode() {
return p.gatewayClients
}
if len(p.gatewayClients) == 0 {
return []*adminapi.Client{}
}
return p.gatewayClients[:1]
}

// mockKongLastValidConfigFetcher is a mock implementation of FallbackConfigGenerator interface.
type mockFallbackConfigGenerator struct {
GenerateResult store.CacheStores
Expand Down Expand Up @@ -261,9 +236,9 @@ func TestKongClientUpdate_AllExpectedClientsAreCalledAndErrorIsPropagated(t *tes

for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
clientsProvider := &mockGatewayClientsProvider{
gatewayClients: tc.gatewayClients,
konnectClient: tc.konnectClient,
clientsProvider := &mocks.MockGatewayClientsProvider{
GatewayClientList: tc.gatewayClients,
KonnectClientInstance: tc.konnectClient,
}
updateStrategyResolver := mocks.NewUpdateStrategyResolver()
for _, url := range tc.errorOnUpdateForURLs {
Expand Down Expand Up @@ -303,12 +278,12 @@ func TestKongClientUpdate_AllExpectedClientsAreCalledAndErrorIsPropagated(t *tes
}

func TestKongClientUpdate_WhenNoChangeInConfigNoClientGetsCalled(t *testing.T) {
clientsProvider := &mockGatewayClientsProvider{
gatewayClients: []*adminapi.Client{
clientsProvider := &mocks.MockGatewayClientsProvider{
GatewayClientList: []*adminapi.Client{
mustSampleGatewayClient(t),
mustSampleGatewayClient(t),
},
konnectClient: mustSampleKonnectClient(t),
KonnectClientInstance: mustSampleKonnectClient(t),
}
updateStrategyResolver := mocks.NewUpdateStrategyResolver()

Expand Down Expand Up @@ -452,9 +427,9 @@ func TestKongClientUpdate_ConfigStatusIsNotified(t *testing.T) {
testKonnectClient = mustSampleKonnectClient(t)
testGatewayClient = mustSampleGatewayClient(t)

clientsProvider = &mockGatewayClientsProvider{
gatewayClients: []*adminapi.Client{testGatewayClient},
konnectClient: testKonnectClient,
clientsProvider = &mocks.MockGatewayClientsProvider{
GatewayClientList: []*adminapi.Client{testGatewayClient},
KonnectClientInstance: testKonnectClient,
}

configChangeDetector = mocks.ConfigurationChangeDetector{ConfigurationChanged: true}
Expand Down Expand Up @@ -548,8 +523,8 @@ func TestKongClientUpdate_ConfigStatusIsNotified(t *testing.T) {

func TestKongClient_ApplyConfigurationEvents(t *testing.T) {
testGatewayClient := mustSampleGatewayClient(t)
clientsProvider := &mockGatewayClientsProvider{
gatewayClients: []*adminapi.Client{testGatewayClient},
clientsProvider := &mocks.MockGatewayClientsProvider{
GatewayClientList: []*adminapi.Client{testGatewayClient},
}
updateStrategyResolver := mocks.NewUpdateStrategyResolver()
configChangeDetector := mocks.ConfigurationChangeDetector{ConfigurationChanged: true}
Expand Down Expand Up @@ -723,8 +698,8 @@ func TestKongClient_KubernetesEvents(t *testing.T) {
eventRecorder := mocks.NewEventRecorder()
lastValidConfigFetcher := &mockKongLastValidConfigFetcher{}
testGatewayClient := mustSampleGatewayClient(t)
clientsProvider := &mockGatewayClientsProvider{
gatewayClients: []*adminapi.Client{testGatewayClient},
clientsProvider := &mocks.MockGatewayClientsProvider{
GatewayClientList: []*adminapi.Client{testGatewayClient},
}
kongClient := setupTestKongClient(t, updateStrategyResolver, clientsProvider, configChangeDetector, configBuilder, eventRecorder, lastValidConfigFetcher)
kongClient.kongConfig.FallbackConfiguration = tc.fallbackConfiguration
Expand Down Expand Up @@ -787,9 +762,9 @@ func TestKongClient_EmptyConfigUpdate(t *testing.T) {
testKonnectClient = mustSampleKonnectClient(t)
testGatewayClient = mustSampleGatewayClient(t)

clientsProvider = &mockGatewayClientsProvider{
gatewayClients: []*adminapi.Client{testGatewayClient},
konnectClient: testKonnectClient,
clientsProvider = &mocks.MockGatewayClientsProvider{
GatewayClientList: []*adminapi.Client{testGatewayClient},
KonnectClientInstance: testKonnectClient,
}

updateStrategyResolver = mocks.NewUpdateStrategyResolver()
Expand Down Expand Up @@ -852,7 +827,7 @@ func TestKongClient_EmptyConfigUpdate(t *testing.T) {
func setupTestKongClient(
t *testing.T,
updateStrategyResolver *mocks.UpdateStrategyResolver,
clientsProvider *mockGatewayClientsProvider,
clientsProvider *mocks.MockGatewayClientsProvider,
configChangeDetector sendconfig.ConfigurationChangeDetector,
configBuilder *mockKongConfigBuilder,
eventRecorder record.EventRecorder,
Expand Down Expand Up @@ -900,7 +875,7 @@ func attachKonnectConfigSynchronizer(
t *testing.T,
kc *KongClient,
updateStrategyResolver *mocks.UpdateStrategyResolver,
clientsProvider *mockGatewayClientsProvider,
clientsProvider *mocks.MockGatewayClientsProvider,
configChangeDetector sendconfig.ConfigurationChangeDetector,
configStatusNotifier clients.ConfigStatusNotifier,
) {
Expand All @@ -911,7 +886,7 @@ func attachKonnectConfigSynchronizer(
logr.Discard(),
config,
testKonnectUploadPeriod,
clientsProvider.KonnectClient(),
clientsProvider,
updateStrategyResolver,
configChangeDetector,
configStatusNotifier,
Expand Down Expand Up @@ -966,8 +941,8 @@ func TestKongClientUpdate_FetchStoreAndPushLastValidConfig(t *testing.T) {
var (
ctx = context.Background()

clientsProvider = &mockGatewayClientsProvider{
gatewayClients: []*adminapi.Client{
clientsProvider = &mocks.MockGatewayClientsProvider{
GatewayClientList: []*adminapi.Client{
mustSampleGatewayClient(t),
mustSampleGatewayClient(t),
},
Expand Down Expand Up @@ -1061,8 +1036,8 @@ func TestKongClientUpdate_FetchStoreAndPushLastValidConfig(t *testing.T) {
t.Run(tc.name, func(t *testing.T) {
updateStrategyResolver := mocks.NewUpdateStrategyResolver()
for range tc.gatewayFailuresCount {
updateStrategyResolver.ReturnSpecificErrorOnUpdate(clientsProvider.gatewayClients[0].BaseRootURL(), tc.errorOnGatewayFailures)
updateStrategyResolver.ReturnSpecificErrorOnUpdate(clientsProvider.gatewayClients[1].BaseRootURL(), tc.errorOnGatewayFailures)
updateStrategyResolver.ReturnSpecificErrorOnUpdate(clientsProvider.GatewayClientList[0].BaseRootURL(), tc.errorOnGatewayFailures)
updateStrategyResolver.ReturnSpecificErrorOnUpdate(clientsProvider.GatewayClientList[1].BaseRootURL(), tc.errorOnGatewayFailures)
}

kongRawStateGetter := &mockKongLastValidConfigFetcher{
Expand Down Expand Up @@ -1094,9 +1069,9 @@ func TestKongClientUpdate_FetchStoreAndPushLastValidConfig(t *testing.T) {

func TestKongClientUpdate_KonnectUpdatesAreSanitized(t *testing.T) {
ctx := context.Background()
clientsProvider := &mockGatewayClientsProvider{
gatewayClients: []*adminapi.Client{mustSampleGatewayClient(t)},
konnectClient: mustSampleKonnectClient(t),
clientsProvider := &mocks.MockGatewayClientsProvider{
GatewayClientList: []*adminapi.Client{mustSampleGatewayClient(t)},
KonnectClientInstance: mustSampleKonnectClient(t),
}
updateStrategyResolver := mocks.NewUpdateStrategyResolver()
configChangeDetector := mocks.ConfigurationChangeDetector{ConfigurationChanged: true}
Expand Down Expand Up @@ -1125,7 +1100,7 @@ func TestKongClientUpdate_KonnectUpdatesAreSanitized(t *testing.T) {
attachKonnectConfigSynchronizer(ctx, t, kongClient, updateStrategyResolver, clientsProvider, configChangeDetector, clients.NoOpConfigStatusNotifier{})
require.NoError(t, kongClient.Update(ctx))

konnectContent := updateStrategyResolver.EventuallyGetLastUpdatedContentForURL(t, clientsProvider.konnectClient.BaseRootURL(), testKonenctUploadWait, testKonnectUploadPeriod)
konnectContent := updateStrategyResolver.EventuallyGetLastUpdatedContentForURL(t, clientsProvider.KonnectClientInstance.BaseRootURL(), testKonenctUploadWait, testKonnectUploadPeriod)
require.Len(t, konnectContent.Content.Certificates, 1, "expected Konnect to have 1 certificate")
cert := konnectContent.Content.Certificates[0]
require.NotNil(t, cert.Key, "expected Konnect to have certificate key")
Expand Down Expand Up @@ -1169,9 +1144,9 @@ func TestKongClient_FallbackConfiguration_SuccessfulRecovery(t *testing.T) {
fallbackConfigGenerator := newMockFallbackConfigGenerator()
gwClient := mustSampleGatewayClient(t)
konnectClient := mustSampleKonnectClient(t)
clientsProvider := &mockGatewayClientsProvider{
gatewayClients: []*adminapi.Client{gwClient},
konnectClient: konnectClient,
clientsProvider := &mocks.MockGatewayClientsProvider{
GatewayClientList: []*adminapi.Client{gwClient},
KonnectClientInstance: konnectClient,
}
kongClient, err := NewKongClient(
zapr.NewLogger(zap.NewNop()),
Expand Down Expand Up @@ -1296,9 +1271,9 @@ func TestKongClient_FallbackConfiguration_SkipsUpdateWhenInSync(t *testing.T) {
ctx := context.Background()
gwClient := mustSampleGatewayClient(t)
konnectClient := mustSampleKonnectClient(t)
clientsProvider := &mockGatewayClientsProvider{
gatewayClients: []*adminapi.Client{gwClient},
konnectClient: konnectClient,
clientsProvider := &mocks.MockGatewayClientsProvider{
GatewayClientList: []*adminapi.Client{gwClient},
KonnectClientInstance: konnectClient,
}
updateStrategyResolver := mocks.NewUpdateStrategyResolver()
configChangeDetector := mocks.ConfigurationChangeDetector{ConfigurationChanged: true}
Expand Down Expand Up @@ -1357,7 +1332,7 @@ func TestKongClient_FallbackConfiguration_SkipsUpdateWhenInSync(t *testing.T) {
newGwClient := mustSampleGatewayClient(t)
t.Run("when new client is discovered, it is updated", func(t *testing.T) {
t.Log("Injecting a new client to the provider")
clientsProvider.gatewayClients = append(clientsProvider.gatewayClients, newGwClient)
clientsProvider.GatewayClientList = append(clientsProvider.GatewayClientList, newGwClient)

t.Log("Calling KongClient.Update again")
require.NoError(t, kongClient.Update(ctx))
Expand Down Expand Up @@ -1401,7 +1376,7 @@ func TestKongClient_FallbackConfiguration_SkipsUpdateWhenInSync(t *testing.T) {
anotherNewGwClient := mustSampleGatewayClient(t)
t.Run("when fallback was used before and config is still broken, after discovering a new client, all clients are updated", func(t *testing.T) {
t.Log("Adding a new client to the provider")
clientsProvider.gatewayClients = append(clientsProvider.gatewayClients, anotherNewGwClient)
clientsProvider.GatewayClientList = append(clientsProvider.GatewayClientList, anotherNewGwClient)
updateStrategyResolver.ReturnSpecificErrorOnUpdate(gwClient.BaseRootURL(), sendconfig.NewUpdateErrorWithoutResponseBody(
[]failures.ResourceFailure{
lo.Must(failures.NewResourceFailure("violated constraint", someConsumer(t, "invalid"))),
Expand Down Expand Up @@ -1442,9 +1417,9 @@ func TestKongClient_FallbackConfiguration_FailedRecovery(t *testing.T) {
ctx := context.Background()
gwClient := mustSampleGatewayClient(t)
konnectClient := mustSampleKonnectClient(t)
clientsProvider := &mockGatewayClientsProvider{
gatewayClients: []*adminapi.Client{gwClient},
konnectClient: konnectClient,
clientsProvider := &mocks.MockGatewayClientsProvider{
GatewayClientList: []*adminapi.Client{gwClient},
KonnectClientInstance: konnectClient,
}
updateStrategyResolver := mocks.NewUpdateStrategyResolver()
configChangeDetector := mocks.ConfigurationChangeDetector{ConfigurationChanged: true}
Expand Down Expand Up @@ -1563,9 +1538,9 @@ func TestKongClient_LastValidCacheSnapshot(t *testing.T) {
t.Run(tc.name, func(t *testing.T) {
testKonnectClient := mustSampleKonnectClient(t)
testGatewayClient := mustSampleGatewayClient(t)
clientsProvider := &mockGatewayClientsProvider{
gatewayClients: []*adminapi.Client{testGatewayClient},
konnectClient: testKonnectClient,
clientsProvider := &mocks.MockGatewayClientsProvider{
GatewayClientList: []*adminapi.Client{testGatewayClient},
KonnectClientInstance: testKonnectClient,
}

kongClient, err := NewKongClient(
Expand Down Expand Up @@ -1616,11 +1591,11 @@ func cacheStoresFromObjs(t *testing.T, objs ...runtime.Object) store.CacheStores
}

func TestKongClient_ConfigDumpSanitization(t *testing.T) {
clientsProvider := &mockGatewayClientsProvider{
gatewayClients: []*adminapi.Client{
clientsProvider := &mocks.MockGatewayClientsProvider{
GatewayClientList: []*adminapi.Client{
mustSampleGatewayClient(t),
},
konnectClient: mustSampleKonnectClient(t),
KonnectClientInstance: mustSampleKonnectClient(t),
}
updateStrategyResolver := mocks.NewUpdateStrategyResolver()
configChangeDetector := mocks.ConfigurationChangeDetector{ConfigurationChanged: true}
Expand Down Expand Up @@ -1762,8 +1737,8 @@ func TestKongClient_RecoveringFromGatewaySyncError(t *testing.T) {
gwClients[i] = mustSampleGatewayClient(t)
updateStrategyResolver.ReturnSpecificErrorOnUpdate(gwClients[i].BaseRootURL(), tc.errorsFromGateways[i])
}
clientsProvider := &mockGatewayClientsProvider{
gatewayClients: gwClients,
clientsProvider := &mocks.MockGatewayClientsProvider{
GatewayClientList: gwClients,
}

lastValidConfigFetcher := &mockKongLastValidConfigFetcher{}
Expand Down
8 changes: 4 additions & 4 deletions internal/konnect/config_synchronizer.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ type ConfigSynchronizer struct {
logger logr.Logger
syncTicker *time.Ticker
kongConfig sendconfig.Config
konnectClient *adminapi.KonnectClient
clientsProvider clients.AdminAPIClientsProvider
metricsRecorder metrics.Recorder
updateStrategyResolver sendconfig.UpdateStrategyResolver
configChangeDetector sendconfig.ConfigurationChangeDetector
Expand All @@ -44,7 +44,7 @@ func NewConfigSynchronizer(
logger logr.Logger,
kongConfig sendconfig.Config,
configUploadPeriod time.Duration,
konnectClient *adminapi.KonnectClient,
clientsProvider clients.AdminAPIClientsProvider,
updateStrategyResolver sendconfig.UpdateStrategyResolver,
configChangeDetector sendconfig.ConfigurationChangeDetector,
configStatusNotifier clients.ConfigStatusNotifier,
Expand All @@ -54,7 +54,7 @@ func NewConfigSynchronizer(
logger: logger,
syncTicker: time.NewTicker(configUploadPeriod),
kongConfig: kongConfig,
konnectClient: konnectClient,
clientsProvider: clientsProvider,
metricsRecorder: metricsRecorder,
updateStrategyResolver: updateStrategyResolver,
configChangeDetector: configChangeDetector,
Expand Down Expand Up @@ -96,7 +96,7 @@ func (s *ConfigSynchronizer) runKonnectUpdateServer(ctx context.Context) {
s.syncTicker.Stop()
case <-s.syncTicker.C:
s.logger.Info("Start uploading to Konnect")
client := s.konnectClient
client := s.clientsProvider.KonnectClient()
if client == nil {
s.logger.Info("Konnect client not ready, skipping")
continue
Expand Down
5 changes: 4 additions & 1 deletion internal/konnect/config_synchronizer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,12 +62,15 @@ func TestConfigSynchronizer_GetTargetContentCopy(t *testing.T) {
func TestConfigSynchronizer_RunKonnectUpdateServer(t *testing.T) {
sendConfigPeriod := 10 * time.Millisecond
testKonnectClient := mustSampleKonnectClient(t)
clientsProvider := &mocks.MockGatewayClientsProvider{
KonnectClientInstance: testKonnectClient,
}
resolver := mocks.NewUpdateStrategyResolver()
log := logr.Discard()
s := &ConfigSynchronizer{
logger: logr.Discard(),
syncTicker: time.NewTicker(sendConfigPeriod),
konnectClient: testKonnectClient,
clientsProvider: clientsProvider,
metricsRecorder: mocks.MetricsRecorder{},
updateStrategyResolver: resolver,
configChangeDetector: sendconfig.NewDefaultConfigurationChangeDetector(log),
Expand Down
2 changes: 1 addition & 1 deletion internal/manager/setup.go
Original file line number Diff line number Diff line change
Expand Up @@ -518,7 +518,7 @@ func setupKonnectConfigSynchronizer(
ctrl.LoggerFrom(ctx).WithName("konnect-config-synchronizer"),
kongConfig,
configUploadPeriod,
clientsProvider.KonnectClient(),
clientsProvider,
updateStrategyResolver,
sendconfig.NewDefaultConfigurationChangeDetector(logger),
configStatusNotifier,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,11 +29,7 @@ func TestHTTPRouteFilterRequestRedirect(t *testing.T) {
New("essentials").
WithLabel(testlabels.NetworkingFamily, testlabels.NetworkingFamilyGatewayAPI).
WithLabel(testlabels.Kind, testlabels.KindHTTPRoute).
WithSetup("deploy Kong addon", featureSetup(
withKongProxyEnvVars(map[string]string{
"PROXY_LISTEN": `0.0.0.0:8000 http2\, 0.0.0.0:8443 http2 ssl`,
}),
)).
WithSetup("deploy Kong addon", featureSetup()).
WithSetup("deploying a gateway and a backend `httpbin` service", func(ctx context.Context, t *testing.T, _ *envconf.Config) context.Context {
cleaner := GetFromCtxForT[*clusters.Cleaner](ctx, t)
cluster := GetClusterFromCtx(ctx)
Expand Down
31 changes: 31 additions & 0 deletions test/mocks/admin_api_clients_provider.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
package mocks

import (
"github.com/kong/kubernetes-ingress-controller/v3/internal/adminapi"
dpconf "github.com/kong/kubernetes-ingress-controller/v3/internal/dataplane/config"
)

// MockGatewayClientsProvider is a mock implementation of dataplane.AdminAPIClientsProvider.
type MockGatewayClientsProvider struct {
GatewayClientList []*adminapi.Client
KonnectClientInstance *adminapi.KonnectClient
DBMode dpconf.DBMode
}

func (p *MockGatewayClientsProvider) KonnectClient() *adminapi.KonnectClient {
return p.KonnectClientInstance
}

func (p *MockGatewayClientsProvider) GatewayClients() []*adminapi.Client {
return p.GatewayClientList
}

func (p *MockGatewayClientsProvider) GatewayClientsToConfigure() []*adminapi.Client {
if p.DBMode.IsDBLessMode() {
return p.GatewayClientList
}
if len(p.GatewayClientList) == 0 {
return []*adminapi.Client{}
}
return p.GatewayClientList[:1]
}
Loading