Fix disabling keepalives using new syntax

Author: swiatekm

config/confighttp/deprecated.go

@@ -93,6 +93,13 @@ func applyLegacyClientKeepalive(conf *confmap.Conf, keepalive *configoptional.Op
 		*keepalive = configoptional.None[KeepaliveClientConfig]()
 		return
 	}
+	// If no legacy fields are present, defer entirely to whatever the new `keepalive`
+	// section (or its absence) already resolved to. Using both legacy fields and the
+	// new section simultaneously is rejected by collectLegacyKeepaliveWarnings before
+	// this function is called, so that case cannot arise here.
+	if !conf.IsSet("idle_conn_timeout") && !conf.IsSet("max_idle_conns") && !conf.IsSet("max_idle_conns_per_host") {
+		return
+	}
 	if !keepalive.HasValue() {
 		*keepalive = configoptional.Some(KeepaliveClientConfig{})
 	}
@@ -155,10 +162,15 @@ func applyLegacyServerKeepalive(conf *confmap.Conf, keepalive *configoptional.Op
 		*keepalive = configoptional.None[KeepaliveServerConfig]()
 		return
 	}
+	// If no legacy fields are present, defer entirely to whatever the new `keepalive`
+	// section (or its absence) already resolved to. Using both legacy fields and the
+	// new section simultaneously is rejected by collectLegacyKeepaliveWarnings before
+	// this function is called, so that case cannot arise here.
+	if !conf.IsSet("idle_timeout") {
+		return
+	}
 	if !keepalive.HasValue() {
 		*keepalive = configoptional.Some(KeepaliveServerConfig{})
 	}
-	if conf.IsSet("idle_timeout") {
-		keepalive.Get().IdleTimeout = legacy.IdleTimeout
-	}
+	keepalive.Get().IdleTimeout = legacy.IdleTimeout
 }

config/confighttp/deprecated_test.go

@@ -163,10 +163,10 @@ func TestApplyLegacyClientKeepalive(t *testing.T) {
 			}),
 		},
 		{
-			name:    "initializes empty keepalive when none and not disabled",
+			name:    "no legacy fields leaves None keepalive unchanged",
 			input:   map[string]any{},
 			initial: configoptional.None[KeepaliveClientConfig](),
-			want:    configoptional.Some(KeepaliveClientConfig{}),
+			want:    configoptional.None[KeepaliveClientConfig](),
 		},
 	}
 
@@ -214,11 +214,11 @@ func TestApplyLegacyServerKeepalive(t *testing.T) {
 			want:    configoptional.Some(KeepaliveServerConfig{IdleTimeout: 120 * time.Second}),
 		},
 		{
-			name:    "initializes empty keepalive when none and enabled",
+			name:    "no legacy fields leaves None keepalive unchanged",
 			input:   map[string]any{},
 			legacy:  legacyServerKeepaliveFields{KeepAlivesEnabled: true},
 			initial: configoptional.None[KeepaliveServerConfig](),
-			want:    configoptional.Some(KeepaliveServerConfig{}),
+			want:    configoptional.None[KeepaliveServerConfig](),
 		},
 	}
 

config/confighttp/testdata/config/client/keepalive_disabled.yaml

@@ -0,0 +1,3 @@
+endpoint: "http://localhost:4318"
+keepalive:
+  enabled: false

config/confighttp/testdata/config/client/legacy_with_keepalive_disabled.yaml

@@ -0,0 +1,4 @@
+endpoint: "http://localhost:4318"
+disable_keep_alives: true
+keepalive:
+  enabled: false

config/confighttp/testdata/config/server/keepalive_disabled.yaml

@@ -0,0 +1,3 @@
+endpoint: "0.0.0.0:4318"
+keepalive:
+  enabled: false

config/confighttp/testdata/config/server/legacy_with_keepalive_disabled.yaml

@@ -0,0 +1,4 @@
+endpoint: "0.0.0.0:4318"
+keep_alives_enabled: false
+keepalive:
+  enabled: false

config/confighttp/unmarshal_test.go

@@ -66,6 +66,16 @@ func TestClientConfigUnmarshal(t *testing.T) {
 			configFile:  "client/mixed_fields_error.yaml",
 			expectError: true,
 		},
+		{
+			name:       "keepalive_disabled",
+			configFile: "client/keepalive_disabled.yaml",
+			want:       configoptional.None[KeepaliveClientConfig](),
+		},
+		{
+			name:        "legacy_with_keepalive_disabled",
+			configFile:  "client/legacy_with_keepalive_disabled.yaml",
+			expectError: true,
+		},
 		{
 			name:       "no_keepalive_fields",
 			configFile: "client/no_keepalive_fields.yaml",
@@ -161,6 +171,16 @@ func TestServerConfigUnmarshal(t *testing.T) {
 			configFile:  "server/mixed_fields_error.yaml",
 			expectError: true,
 		},
+		{
+			name:       "keepalive_disabled",
+			configFile: "server/keepalive_disabled.yaml",
+			want:       configoptional.None[KeepaliveServerConfig](),
+		},
+		{
+			name:        "legacy_with_keepalive_disabled",
+			configFile:  "server/legacy_with_keepalive_disabled.yaml",
+			expectError: true,
+		},
 		{
 			name:       "no_keepalive_fields",
 			configFile: "server/no_keepalive_fields.yaml",