Skip to content

Commit 3447c97

Browse files
author
Jim Ryan
committed
error messages if config is invalid
1 parent dd7897a commit 3447c97

File tree

4 files changed

+17
-18
lines changed

4 files changed

+17
-18
lines changed

‎cmd/nginx-ingress/main.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -865,7 +865,7 @@ func processConfigMaps(kubeClient *kubernetes.Clientset, cfgParams *configs.Conf
865865
if err != nil {
866866
nl.Fatalf(l, "Error when getting %v: %v", *nginxConfigMaps, err)
867867
}
868-
cfgParams = configs.ParseConfigMap(cfgParams.Context, cfm, *nginxPlus, *appProtect, *appProtectDos, *enableTLSPassthrough, eventLog)
868+
cfgParams, _ = configs.ParseConfigMap(cfgParams.Context, cfm, *nginxPlus, *appProtect, *appProtectDos, *enableTLSPassthrough, eventLog)
869869
if cfgParams.MainServerSSLDHParamFileContent != nil {
870870
fileName, err := nginxManager.CreateDHParam(*cfgParams.MainServerSSLDHParamFileContent)
871871
if err != nil {

‎internal/configs/configmaps.go‎

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ const (
2020
// ParseConfigMap parses ConfigMap into ConfigParams.
2121
//
2222
//nolint:gocyclo
23-
func ParseConfigMap(ctx context.Context, cfgm *v1.ConfigMap, nginxPlus bool, hasAppProtect bool, hasAppProtectDos bool, hasTLSPassthrough bool, eventLog record.EventRecorder) *ConfigParams {
23+
func ParseConfigMap(ctx context.Context, cfgm *v1.ConfigMap, nginxPlus bool, hasAppProtect bool, hasAppProtectDos bool, hasTLSPassthrough bool, eventLog record.EventRecorder) (*ConfigParams, bool) {
2424
l := nl.LoggerFromContext(ctx)
2525
cfgParams := NewDefaultConfigParams(ctx, nginxPlus)
2626
configOk := true
@@ -652,13 +652,7 @@ func ParseConfigMap(ctx context.Context, cfgm *v1.ConfigMap, nginxPlus bool, has
652652
}
653653
}
654654

655-
if configOk {
656-
eventLog.Event(cfgm, v1.EventTypeNormal, "Updated", fmt.Sprintf("ConfigMap %s/%s updated without error", cfgm.GetNamespace(), cfgm.GetName()))
657-
} else {
658-
eventLog.Event(cfgm, v1.EventTypeWarning, "UpdatedWithError", fmt.Sprintf("ConfigMap %s/%s updated with errors. Ignoring invalid values", cfgm.GetNamespace(), cfgm.GetName()))
659-
}
660-
661-
return cfgParams
655+
return cfgParams, configOk
662656
}
663657

664658
// GenerateNginxMainConfig generates MainConfig.

‎internal/configs/configmaps_test.go‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ func TestParseConfigMapWithAppProtectCompressedRequestsAction(t *testing.T) {
4646
"app-protect-compressed-requests-action": test.action,
4747
},
4848
}
49-
result := ParseConfigMap(context.Background(), cm, nginxPlus, hasAppProtect, hasAppProtectDos, hasTLSPassthrough, makeEventLogger())
49+
result, _ := ParseConfigMap(context.Background(), cm, nginxPlus, hasAppProtect, hasAppProtectDos, hasTLSPassthrough, makeEventLogger())
5050
if result.MainAppProtectCompressedRequestsAction != test.expect {
5151
t.Errorf("ParseConfigMap() returned %q but expected %q for the case %s", result.MainAppProtectCompressedRequestsAction, test.expect, test.msg)
5252
}
@@ -115,7 +115,7 @@ func TestParseConfigMapWithAppProtectReconnectPeriod(t *testing.T) {
115115
"app-protect-reconnect-period-seconds": test.period,
116116
},
117117
}
118-
result := ParseConfigMap(context.Background(), cm, nginxPlus, hasAppProtect, hasAppProtectDos, hasTLSPassthrough, makeEventLogger())
118+
result, _ := ParseConfigMap(context.Background(), cm, nginxPlus, hasAppProtect, hasAppProtectDos, hasTLSPassthrough, makeEventLogger())
119119
if result.MainAppProtectReconnectPeriod != test.expect {
120120
t.Errorf("ParseConfigMap() returned %q but expected %q for the case %s", result.MainAppProtectReconnectPeriod, test.expect, test.msg)
121121
}
@@ -156,7 +156,7 @@ func TestParseConfigMapWithTLSPassthroughProxyProtocol(t *testing.T) {
156156
"real-ip-header": test.realIPheader,
157157
},
158158
}
159-
result := ParseConfigMap(context.Background(), cm, nginxPlus, hasAppProtect, hasAppProtectDos, hasTLSPassthrough, makeEventLogger())
159+
result, _ := ParseConfigMap(context.Background(), cm, nginxPlus, hasAppProtect, hasAppProtectDos, hasTLSPassthrough, makeEventLogger())
160160
if result.RealIPHeader != test.want {
161161
t.Errorf("want %q, got %q", test.want, result.RealIPHeader)
162162
}
@@ -198,7 +198,7 @@ func TestParseConfigMapWithoutTLSPassthroughProxyProtocol(t *testing.T) {
198198
"real-ip-header": test.realIPheader,
199199
},
200200
}
201-
result := ParseConfigMap(context.Background(), cm, nginxPlus, hasAppProtect, hasAppProtectDos, hasTLSPassthrough, makeEventLogger())
201+
result, _ := ParseConfigMap(context.Background(), cm, nginxPlus, hasAppProtect, hasAppProtectDos, hasTLSPassthrough, makeEventLogger())
202202
if result.RealIPHeader != test.want {
203203
t.Errorf("want %q, got %q", test.want, result.RealIPHeader)
204204
}
@@ -245,7 +245,7 @@ func TestParseConfigMapAccessLog(t *testing.T) {
245245
"access-log-off": test.accessLogOff,
246246
},
247247
}
248-
result := ParseConfigMap(context.Background(), cm, nginxPlus, hasAppProtect, hasAppProtectDos, hasTLSPassthrough, makeEventLogger())
248+
result, _ := ParseConfigMap(context.Background(), cm, nginxPlus, hasAppProtect, hasAppProtectDos, hasTLSPassthrough, makeEventLogger())
249249
if result.MainAccessLog != test.want {
250250
t.Errorf("want %q, got %q", test.want, result.MainAccessLog)
251251
}
@@ -277,7 +277,7 @@ func TestParseConfigMapAccessLogDefault(t *testing.T) {
277277
"access-log-off": "False",
278278
},
279279
}
280-
result := ParseConfigMap(context.Background(), cm, nginxPlus, hasAppProtect, hasAppProtectDos, hasTLSPassthrough, makeEventLogger())
280+
result, _ := ParseConfigMap(context.Background(), cm, nginxPlus, hasAppProtect, hasAppProtectDos, hasTLSPassthrough, makeEventLogger())
281281
if result.MainAccessLog != test.want {
282282
t.Errorf("want %q, got %q", test.want, result.MainAccessLog)
283283
}

‎internal/k8s/controller.go‎

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,7 @@ import (
5454
"github.com/nginxinc/kubernetes-ingress/internal/metrics/collectors"
5555

5656
api_v1 "k8s.io/api/core/v1"
57+
v1 "k8s.io/api/core/v1"
5758
discovery_v1 "k8s.io/api/discovery/v1"
5859
networking "k8s.io/api/networking/v1"
5960
meta_v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -841,9 +842,10 @@ func (lbc *LoadBalancerController) createExtendedResources(resources []Resource)
841842
func (lbc *LoadBalancerController) updateAllConfigs() {
842843
ctx := nl.ContextWithLogger(context.Background(), lbc.Logger)
843844
cfgParams := configs.NewDefaultConfigParams(ctx, lbc.isNginxPlus)
845+
var isNGINXConfigValid bool
844846

845847
if lbc.configMap != nil {
846-
cfgParams = configs.ParseConfigMap(ctx, lbc.configMap, lbc.isNginxPlus, lbc.appProtectEnabled, lbc.appProtectDosEnabled, lbc.configuration.isTLSPassthroughEnabled, lbc.recorder)
848+
cfgParams, isNGINXConfigValid = configs.ParseConfigMap(ctx, lbc.configMap, lbc.isNginxPlus, lbc.appProtectEnabled, lbc.appProtectDosEnabled, lbc.configuration.isTLSPassthroughEnabled, lbc.recorder)
847849
}
848850

849851
resources := lbc.configuration.GetResources()
@@ -869,8 +871,11 @@ func (lbc *LoadBalancerController) updateAllConfigs() {
869871
}
870872

871873
if lbc.configMap != nil {
872-
key := getResourceKey(&lbc.configMap.ObjectMeta)
873-
lbc.recorder.Eventf(lbc.configMap, eventType, eventTitle, "Configuration from %v was updated %s", key, eventWarningMessage)
874+
if isNGINXConfigValid {
875+
lbc.recorder.Event(lbc.configMap, v1.EventTypeNormal, "Updated", fmt.Sprintf("ConfigMap %s/%s updated without error", lbc.configMap.GetNamespace(), lbc.configMap.GetName()))
876+
} else {
877+
lbc.recorder.Event(lbc.configMap, v1.EventTypeWarning, "UpdatedWithError", fmt.Sprintf("ConfigMap %s/%s updated with errors. Ignoring invalid values", lbc.configMap.GetNamespace(), lbc.configMap.GetName()))
878+
}
874879
}
875880

876881
gc := lbc.configuration.GetGlobalConfiguration()

0 commit comments

Comments
 (0)