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

Add events to configmap #6819

Merged
merged 6 commits into from
Nov 18, 2024
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)
pdabelf5 marked this conversation as resolved.
Show resolved Hide resolved
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