Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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
6 changes: 3 additions & 3 deletions cmd/nginx-ingress/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ func main() {
mustProcessGlobalConfiguration(ctx)

cfgParams := configs.NewDefaultConfigParams(ctx, *nginxPlus)
cfgParams = processConfigMaps(kubeClient, cfgParams, nginxManager, templateExecutor)
cfgParams = processConfigMaps(kubeClient, cfgParams, nginxManager, templateExecutor, eventRecorder)

staticCfgParams := &configs.StaticConfigParams{
DisableIPV6: *disableIPV6,
Expand Down Expand Up @@ -854,7 +854,7 @@ func mustProcessGlobalConfiguration(ctx context.Context) {
}
}

func processConfigMaps(kubeClient *kubernetes.Clientset, cfgParams *configs.ConfigParams, nginxManager nginx.Manager, templateExecutor *version1.TemplateExecutor) *configs.ConfigParams {
func processConfigMaps(kubeClient *kubernetes.Clientset, cfgParams *configs.ConfigParams, nginxManager nginx.Manager, templateExecutor *version1.TemplateExecutor, eventLog record.EventRecorder) *configs.ConfigParams {
l := nl.LoggerFromContext(cfgParams.Context)
if *nginxConfigMaps != "" {
ns, name, err := k8s.ParseNamespaceName(*nginxConfigMaps)
Expand All @@ -865,7 +865,7 @@ func processConfigMaps(kubeClient *kubernetes.Clientset, cfgParams *configs.Conf
if err != nil {
nl.Fatalf(l, "Error when getting %v: %v", *nginxConfigMaps, err)
}
cfgParams = configs.ParseConfigMap(cfgParams.Context, cfm, *nginxPlus, *appProtect, *appProtectDos, *enableTLSPassthrough)
cfgParams, _ = configs.ParseConfigMap(cfgParams.Context, cfm, *nginxPlus, *appProtect, *appProtectDos, *enableTLSPassthrough, eventLog)
if cfgParams.MainServerSSLDHParamFileContent != nil {
fileName, err := nginxManager.CreateDHParam(*cfgParams.MainServerSSLDHParamFileContent)
if err != nil {
Expand Down
169 changes: 147 additions & 22 deletions internal/configs/configmaps.go

Large diffs are not rendered by default.

17 changes: 11 additions & 6 deletions internal/configs/configmaps_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"testing"

v1 "k8s.io/api/core/v1"
"k8s.io/client-go/tools/record"
)

func TestParseConfigMapWithAppProtectCompressedRequestsAction(t *testing.T) {
Expand Down Expand Up @@ -45,7 +46,7 @@ func TestParseConfigMapWithAppProtectCompressedRequestsAction(t *testing.T) {
"app-protect-compressed-requests-action": test.action,
},
}
result := ParseConfigMap(context.Background(), cm, nginxPlus, hasAppProtect, hasAppProtectDos, hasTLSPassthrough)
result, _ := ParseConfigMap(context.Background(), cm, nginxPlus, hasAppProtect, hasAppProtectDos, hasTLSPassthrough, makeEventLogger())
if result.MainAppProtectCompressedRequestsAction != test.expect {
t.Errorf("ParseConfigMap() returned %q but expected %q for the case %s", result.MainAppProtectCompressedRequestsAction, test.expect, test.msg)
}
Expand Down Expand Up @@ -114,7 +115,7 @@ func TestParseConfigMapWithAppProtectReconnectPeriod(t *testing.T) {
"app-protect-reconnect-period-seconds": test.period,
},
}
result := ParseConfigMap(context.Background(), cm, nginxPlus, hasAppProtect, hasAppProtectDos, hasTLSPassthrough)
result, _ := ParseConfigMap(context.Background(), cm, nginxPlus, hasAppProtect, hasAppProtectDos, hasTLSPassthrough, makeEventLogger())
if result.MainAppProtectReconnectPeriod != test.expect {
t.Errorf("ParseConfigMap() returned %q but expected %q for the case %s", result.MainAppProtectReconnectPeriod, test.expect, test.msg)
}
Expand Down Expand Up @@ -155,7 +156,7 @@ func TestParseConfigMapWithTLSPassthroughProxyProtocol(t *testing.T) {
"real-ip-header": test.realIPheader,
},
}
result := ParseConfigMap(context.Background(), cm, nginxPlus, hasAppProtect, hasAppProtectDos, hasTLSPassthrough)
result, _ := ParseConfigMap(context.Background(), cm, nginxPlus, hasAppProtect, hasAppProtectDos, hasTLSPassthrough, makeEventLogger())
if result.RealIPHeader != test.want {
t.Errorf("want %q, got %q", test.want, result.RealIPHeader)
}
Expand Down Expand Up @@ -197,7 +198,7 @@ func TestParseConfigMapWithoutTLSPassthroughProxyProtocol(t *testing.T) {
"real-ip-header": test.realIPheader,
},
}
result := ParseConfigMap(context.Background(), cm, nginxPlus, hasAppProtect, hasAppProtectDos, hasTLSPassthrough)
result, _ := ParseConfigMap(context.Background(), cm, nginxPlus, hasAppProtect, hasAppProtectDos, hasTLSPassthrough, makeEventLogger())
if result.RealIPHeader != test.want {
t.Errorf("want %q, got %q", test.want, result.RealIPHeader)
}
Expand Down Expand Up @@ -244,7 +245,7 @@ func TestParseConfigMapAccessLog(t *testing.T) {
"access-log-off": test.accessLogOff,
},
}
result := ParseConfigMap(context.Background(), cm, nginxPlus, hasAppProtect, hasAppProtectDos, hasTLSPassthrough)
result, _ := ParseConfigMap(context.Background(), cm, nginxPlus, hasAppProtect, hasAppProtectDos, hasTLSPassthrough, makeEventLogger())
if result.MainAccessLog != test.want {
t.Errorf("want %q, got %q", test.want, result.MainAccessLog)
}
Expand Down Expand Up @@ -276,10 +277,14 @@ func TestParseConfigMapAccessLogDefault(t *testing.T) {
"access-log-off": "False",
},
}
result := ParseConfigMap(context.Background(), cm, nginxPlus, hasAppProtect, hasAppProtectDos, hasTLSPassthrough)
result, _ := ParseConfigMap(context.Background(), cm, nginxPlus, hasAppProtect, hasAppProtectDos, hasTLSPassthrough, makeEventLogger())
if result.MainAccessLog != test.want {
t.Errorf("want %q, got %q", test.want, result.MainAccessLog)
}
})
}
}

func makeEventLogger() record.EventRecorder {
return record.NewFakeRecorder(1024)
}
10 changes: 7 additions & 3 deletions internal/k8s/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -841,9 +841,10 @@ func (lbc *LoadBalancerController) createExtendedResources(resources []Resource)
func (lbc *LoadBalancerController) updateAllConfigs() {
ctx := nl.ContextWithLogger(context.Background(), lbc.Logger)
cfgParams := configs.NewDefaultConfigParams(ctx, lbc.isNginxPlus)
var isNGINXConfigValid bool

if lbc.configMap != nil {
cfgParams = configs.ParseConfigMap(ctx, lbc.configMap, lbc.isNginxPlus, lbc.appProtectEnabled, lbc.appProtectDosEnabled, lbc.configuration.isTLSPassthroughEnabled)
cfgParams, isNGINXConfigValid = configs.ParseConfigMap(ctx, lbc.configMap, lbc.isNginxPlus, lbc.appProtectEnabled, lbc.appProtectDosEnabled, lbc.configuration.isTLSPassthroughEnabled, lbc.recorder)
}

resources := lbc.configuration.GetResources()
Expand All @@ -869,8 +870,11 @@ func (lbc *LoadBalancerController) updateAllConfigs() {
}

if lbc.configMap != nil {
key := getResourceKey(&lbc.configMap.ObjectMeta)
lbc.recorder.Eventf(lbc.configMap, eventType, eventTitle, "Configuration from %v was updated %s", key, eventWarningMessage)
if isNGINXConfigValid {
lbc.recorder.Event(lbc.configMap, api_v1.EventTypeNormal, "Updated", fmt.Sprintf("ConfigMap %s/%s updated without error", lbc.configMap.GetNamespace(), lbc.configMap.GetName()))
} else {
lbc.recorder.Event(lbc.configMap, api_v1.EventTypeWarning, "UpdatedWithError", fmt.Sprintf("ConfigMap %s/%s updated with errors. Ignoring invalid values", lbc.configMap.GetNamespace(), lbc.configMap.GetName()))
}
}

gc := lbc.configuration.GetGlobalConfiguration()
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
apiVersion: v1
kind: ConfigMap
metadata:
name: nginx-config
namespace: nginx-ingress
data:
proxy-buffering: "invalid" # Invalid boolean
proxy-read-timeout: "60s"
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
apiVersion: v1
kind: ConfigMap
metadata:
name: nginx-config
namespace: nginx-ingress
data:
proxy-buffering: "on"
proxy-read-timeout: "60s"
84 changes: 84 additions & 0 deletions tests/suite/test_virtual_server_configmap_keys.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
from settings import DEPLOYMENTS, TEST_DATA
from suite.utils.resources_utils import (
get_events,
get_events_for_object,
get_file_contents,
get_first_pod_name,
get_pods_amount,
Expand Down Expand Up @@ -124,6 +125,23 @@ def assert_defaults_of_ssl_keys(config):
assert "http2 on;" not in config


def assert_event(event_list, event_type, reason, message_substring):
"""
Assert that an event with specific type, reason, and message substring exists.

:param event_list: List of events
:param event_type: 'Normal' or 'Warning'
:param reason: Event reason
:param message_substring: Substring expected in the event message
"""
for event in event_list:
if event.type == event_type and event.reason == reason and message_substring in event.message:
return
assert (
False
), f"Expected event with type '{event_type}', reason '{reason}', and message containing '{message_substring}' not found."


@pytest.fixture(scope="function")
def clean_up(request, kube_apis, ingress_controller_prerequisites, test_namespace) -> None:
"""
Expand Down Expand Up @@ -404,3 +422,69 @@ def test_ssl_keys(
)
assert_update_event_count_increased(virtual_server_setup, step_2_events, step_1_events)
assert_defaults_of_ssl_keys(step_2_config)

def test_configmap_events(
self,
kube_apis,
ingress_controller_prerequisites,
crd_ingress_controller,
virtual_server_setup,
clean_up,
):
ingress_controller_prerequisites.namespace
configmap_name = ingress_controller_prerequisites.config_map["metadata"]["name"]
configmap_namespace = ingress_controller_prerequisites.config_map["metadata"]["namespace"]

# Step 1: Update ConfigMap with valid parameters
print("Updating ConfigMap with valid parameters")
replace_configmap_from_yaml(
kube_apis.v1,
configmap_name,
configmap_namespace,
f"{TEST_DATA}/virtual-server-configmap-keys/configmap-valid.yaml",
)
wait_before_test(1)

# Get events for the ConfigMap
events = get_events_for_object(
kube_apis.v1,
configmap_namespace,
configmap_name,
)

assert len(events) >= 1

# Assert that the 'updated without error' event is present
assert_event(
events,
"Normal",
"Updated",
f"ConfigMap {configmap_namespace}/{configmap_name} updated without error",
)

# Step 2: Update ConfigMap with invalid parameters
print("Updating ConfigMap with invalid parameters")
replace_configmap_from_yaml(
kube_apis.v1,
configmap_name,
configmap_namespace,
f"{TEST_DATA}/virtual-server-configmap-keys/configmap-invalid.yaml",
)
wait_before_test(1)

# Get events for the ConfigMap
events = get_events_for_object(
kube_apis.v1,
configmap_namespace,
configmap_name,
)

assert len(events) >= 1

# Assert that the 'updated with errors' event is present
assert_event(
events,
"Warning",
"UpdatedWithError",
f"ConfigMap {configmap_namespace}/{configmap_name} updated with errors. Ignoring invalid values",
)
Loading