chore: split config change detector into gw and konnect flavor (#6942)

internal/dataplane/kong_client_golden_test.go

@@ -306,7 +306,7 @@ func runKongClientGoldenTest(t *testing.T, tc kongClientGoldenTestCase) {
 		dpconf.DBModeOff, // Test will run in DB-less mode only for now. In the future, we may want to test DB mode as well.
 		clientsProvider,
 		updateStrategyResolver,
-		sendconfig.NewDefaultConfigurationChangeDetector(logger),
+		sendconfig.NewKongGatewayConfigurationChangeDetector(logger),
 		lastValidConfigFetcher,
 		p,
 		&cacheStores,

internal/dataplane/sendconfig/config_change_detector.go

@@ -18,55 +18,42 @@ const (
 )
 
 type ConfigurationChangeDetector interface {
-	// HasConfigurationChanged verifies whether configuration has changed by comparing
-	// old and new config's SHAs.
-	// In case the SHAs are equal, it still can return true if a client is considered
-	// crashed or just booted up based on its status.
-	// In case the status indicates an empty config and the desired config is also empty
-	// this will return false to prevent continuously sending empty configuration to Gateway.
 	HasConfigurationChanged(
 		ctx context.Context,
 		oldSHA, newSHA []byte,
 		targetConfig *file.Content,
-		client KonnectAwareClient,
 		statusClient StatusClient,
 	) (bool, error)
 }
 
-type KonnectAwareClient interface {
-	IsKonnect() bool
-}
-
 type StatusClient interface {
 	Status(context.Context) (*kong.Status, error)
 }
 
-type DefaultConfigurationChangeDetector struct {
+// KongGatewayConfigurationChangeDetector detects changes in Kong Gateway configuration. Besides comparing SHA hashes,
+// it also checks if the Kong Gateway has no configuration yet or if the configuration to be pushed is empty.
+type KongGatewayConfigurationChangeDetector struct {
 	logger logr.Logger
 }
 
-func NewDefaultConfigurationChangeDetector(logger logr.Logger) *DefaultConfigurationChangeDetector {
-	return &DefaultConfigurationChangeDetector{logger: logger}
+func NewKongGatewayConfigurationChangeDetector(logger logr.Logger) *KongGatewayConfigurationChangeDetector {
+	return &KongGatewayConfigurationChangeDetector{logger: logger}
 }
 
-func (d *DefaultConfigurationChangeDetector) HasConfigurationChanged(
+// HasConfigurationChanged verifies whether configuration has changed by comparing
+// old and new config's SHAs. In case the SHAs are equal, it still can return true if a client is considered  crashed or
+// just booted up based on its status. In case the status indicates an empty config and the desired config is also empty
+// this will return false to prevent continuously sending empty configuration to Gateway.
+func (d *KongGatewayConfigurationChangeDetector) HasConfigurationChanged(
 	ctx context.Context,
 	oldSHA, newSHA []byte,
 	targetConfig *file.Content,
-	client KonnectAwareClient,
 	statusClient StatusClient,
 ) (bool, error) {
 	if !bytes.Equal(oldSHA, newSHA) {
 		return true, nil
 	}
 
-	// In case of Konnect, we skip further steps that are meant to detect Kong instances crash/reset
-	// that are not relevant for Konnect.
-	// We're sure that if oldSHA and newSHA are equal, we are safe to skip the update.
-	if client.IsKonnect() {
-		return false, nil
-	}
-
 	// Check if a Kong instance has no configuration yet (could mean it crashed, was rebooted, etc.).
 	hasNoConfiguration, err := kongHasNoConfiguration(ctx, statusClient)
 	if err != nil {
@@ -86,10 +73,9 @@ func (d *DefaultConfigurationChangeDetector) HasConfigurationChanged(
 	return false, nil
 }
 
-// kongHasNoConfiguration checks Kong's status endpoint and read its config hash.
-// If the config hash reported by Kong is the known empty hash, it's considered crashed.
-// This allows providing configuration to Kong instances that have unexpectedly crashed and
-// lost their configuration.
+// kongHasNoConfiguration checks Kong's status endpoint and read its config hash. If the config hash reported by Kong is
+// the known empty hash, it's considered crashed. This allows providing configuration to Kong instances that have
+// unexpectedly crashed and lost their configuration.
 func kongHasNoConfiguration(ctx context.Context, client StatusClient) (bool, error) {
 	status, err := client.Status(ctx)
 	if err != nil {
@@ -102,3 +88,20 @@ func kongHasNoConfiguration(ctx context.Context, client StatusClient) (bool, err
 
 	return false, nil
 }
+
+// KonnectConfigurationChangeDetector detects changes in Konnect configuration by comparing SHA hashes only.
+type KonnectConfigurationChangeDetector struct{}
+
+func NewKonnectConfigurationChangeDetector() *KonnectConfigurationChangeDetector {
+	return &KonnectConfigurationChangeDetector{}
+}
+
+// HasConfigurationChanged verifies whether configuration has changed by comparing old and new config's SHAs.
+func (d *KonnectConfigurationChangeDetector) HasConfigurationChanged(
+	_ context.Context,
+	oldSHA, newSHA []byte,
+	_ *file.Content,
+	_ StatusClient,
+) (bool, error) {
+	return !bytes.Equal(oldSHA, newSHA), nil
+}

internal/dataplane/sendconfig/config_change_detector_test.go

@@ -14,14 +14,6 @@ import (
 	"github.com/kong/kubernetes-ingress-controller/v3/internal/dataplane/sendconfig"
 )
 
-type konnectAwareClientMock struct {
-	expected bool
-}
-
-func (c konnectAwareClientMock) IsKonnect() bool {
-	return c.expected
-}
-
 type statusClientMock struct {
 	expectedValue *kong.Status
 	expectedError error
@@ -104,15 +96,6 @@ func TestDefaultConfigurationChangeDetector_HasConfigurationChanged(t *testing.T
 			statusSHA:      string(testSHAs[1]),
 			expectedResult: false,
 		},
-		{
-			name:           "oldSHA == newSHA, status would signal crash, but it's konnect",
-			oldSHA:         testSHAs[0],
-			newSHA:         testSHAs[0],
-			targetConfig:   createConfigContent(),
-			statusSHA:      sendconfig.WellKnownInitialHash,
-			isKonnect:      true,
-			expectedResult: false,
-		},
 		{
 			name:         "oldSHA == newSHA, status returns error",
 			oldSHA:       testSHAs[0],
@@ -125,16 +108,15 @@ func TestDefaultConfigurationChangeDetector_HasConfigurationChanged(t *testing.T
 
 	for _, tc := range testCases {
 		t.Run(tc.name, func(t *testing.T) {
-			konnectAwareClient := konnectAwareClientMock{expected: tc.isKonnect}
 			statusClient := statusClientMock{
 				expectedValue: &kong.Status{
 					ConfigurationHash: tc.statusSHA,
 				},
 				expectedError: tc.statusError,
 			}
-			detector := sendconfig.NewDefaultConfigurationChangeDetector(zapr.NewLogger(zap.NewNop()))
+			detector := sendconfig.NewKongGatewayConfigurationChangeDetector(zapr.NewLogger(zap.NewNop()))
 
-			result, err := detector.HasConfigurationChanged(ctx, tc.oldSHA, tc.newSHA, tc.targetConfig, konnectAwareClient, statusClient)
+			result, err := detector.HasConfigurationChanged(ctx, tc.oldSHA, tc.newSHA, tc.targetConfig, statusClient)
 			if tc.expectError {
 				require.Error(t, err)
 				return
@@ -145,3 +127,42 @@ func TestDefaultConfigurationChangeDetector_HasConfigurationChanged(t *testing.T
 		})
 	}
 }
+
+func TestKonnectConfigurationChangeDetector(t *testing.T) {
+	ctx := context.Background()
+	testSHAs := [][]byte{
+		[]byte("2cf24dba5fb0a30e26e83b2ac5b9e29e1b161e5c1fa7425e73043362938b9824"),
+		[]byte("82e35a63ceba37e9646434c5dd412ea577147f1e4a41ccde1614253187e3dbf9"),
+	}
+
+	testCases := []struct {
+		name           string
+		oldSHA, newSHA []byte
+		expectedResult bool
+		expectError    bool
+	}{
+		{
+			name:           "oldSHA != newSHA",
+			oldSHA:         testSHAs[0],
+			newSHA:         testSHAs[1],
+			expectedResult: true,
+		},
+		{
+			name:           "oldSHA == newSHA",
+			oldSHA:         testSHAs[0],
+			newSHA:         testSHAs[0],
+			expectedResult: false,
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			detector := sendconfig.NewKonnectConfigurationChangeDetector()
+
+			// Passing nil for content and status client explicitly, as they are not used in this detector.
+			result, err := detector.HasConfigurationChanged(ctx, tc.oldSHA, tc.newSHA, nil, nil)
+			require.NoError(t, err)
+			require.Equal(t, tc.expectedResult, result)
+		})
+	}
+}

internal/dataplane/sendconfig/sendconfig.go

@@ -60,7 +60,7 @@ func PerformUpdate(
 
 	// disable optimization if reverse sync is enabled
 	if !config.EnableReverseSync {
-		configurationChanged, err := configChangeDetector.HasConfigurationChanged(ctx, oldSHA, newSHA, targetContent, client, client.AdminAPIClient())
+		configurationChanged, err := configChangeDetector.HasConfigurationChanged(ctx, oldSHA, newSHA, targetContent, client.AdminAPIClient())
 		if err != nil {
 			return nil, fmt.Errorf("failed to detect configuration change: %w", err)
 		}

internal/konnect/config_synchronizer_test.go

@@ -41,7 +41,7 @@ func TestConfigSynchronizer_UpdatesKongConfigAccordingly(t *testing.T) {
 			ConfigUploadTicker:     clock.NewTickerWithDuration(testSendConfigPeriod),
 			KonnectClientFactory:   &mocks.KonnectClientFactory{Client: testKonnectClient},
 			UpdateStrategyResolver: resolver,
-			ConfigChangeDetector:   sendconfig.NewDefaultConfigurationChangeDetector(log),
+			ConfigChangeDetector:   sendconfig.NewKongGatewayConfigurationChangeDetector(log),
 			ConfigStatusNotifier:   configStatusNotifier,
 			MetricsRecorder:        &mocks.MetricsRecorder{},
 		},
@@ -141,7 +141,7 @@ func TestConfigSynchronizer_ConfigIsSanitizedWhenConfiguredSo(t *testing.T) {
 			ConfigUploadTicker:     clock.NewTickerWithDuration(testSendConfigPeriod),
 			KonnectClientFactory:   &mocks.KonnectClientFactory{Client: testKonnectClient},
 			UpdateStrategyResolver: resolver,
-			ConfigChangeDetector:   sendconfig.NewDefaultConfigurationChangeDetector(log),
+			ConfigChangeDetector:   sendconfig.NewKongGatewayConfigurationChangeDetector(log),
 			ConfigStatusNotifier:   configStatusNotifier,
 			MetricsRecorder:        &mocks.MetricsRecorder{},
 		},

internal/manager/run.go

@@ -199,7 +199,7 @@ func Run(
 	}
 
 	updateStrategyResolver := sendconfig.NewDefaultUpdateStrategyResolver(kongConfig, logger)
-	configurationChangeDetector := sendconfig.NewDefaultConfigurationChangeDetector(logger)
+	configurationChangeDetector := sendconfig.NewKongGatewayConfigurationChangeDetector(logger)
 	kongConfigFetcher := configfetcher.NewDefaultKongLastGoodConfigFetcher(translatorFeatureFlags.FillIDs, c.KongWorkspace)
 	fallbackConfigGenerator := fallback.NewGenerator(fallback.NewDefaultCacheGraphProvider(), logger)
 	metricsRecorder := metrics.NewGlobalCtrlRuntimeMetricsRecorder()

internal/manager/setup.go

@@ -503,7 +503,7 @@ func setupLicenseGetter(
 	return nil, nil
 }
 
-// setupKonnectConfigSynchronizerWithMgr sets up Konnect config sychronizer and adds it to the manager runnables.
+// setupKonnectConfigSynchronizerWithMgr sets up Konnect config synchronizer and adds it to the manager runnables.
 func setupKonnectConfigSynchronizerWithMgr(
 	ctx context.Context,
 	mgr manager.Manager,
@@ -520,11 +520,9 @@ func setupKonnectConfigSynchronizerWithMgr(
 			ConfigUploadTicker:     clock.NewTickerWithDuration(cfg.Konnect.UploadConfigPeriod),
 			KonnectClientFactory:   adminapi.NewKonnectClientFactory(cfg.Konnect, ctrl.LoggerFrom(ctx).WithName("konnect-client-factory")),
 			UpdateStrategyResolver: updateStrategyResolver,
-			ConfigChangeDetector: sendconfig.NewDefaultConfigurationChangeDetector(
-				ctrl.LoggerFrom(ctx).WithName("konnect-config-change-detector"),
-			),
-			ConfigStatusNotifier: configStatusNotifier,
-			MetricsRecorder:      metricsRecorder,
+			ConfigChangeDetector:   sendconfig.NewKonnectConfigurationChangeDetector(),
+			ConfigStatusNotifier:   configStatusNotifier,
+			MetricsRecorder:        metricsRecorder,
 		},
 	)
 	err := mgr.Add(s)

test/mocks/configuration_change_detector.go

@@ -14,7 +14,7 @@ type ConfigurationChangeDetector struct {
 }
 
 func (m ConfigurationChangeDetector) HasConfigurationChanged(
-	context.Context, []byte, []byte, *file.Content, sendconfig.KonnectAwareClient, sendconfig.StatusClient,
+	context.Context, []byte, []byte, *file.Content, sendconfig.StatusClient,
 ) (bool, error) {
 	return m.ConfigurationChanged, nil
 }