Skip to content

Commit df087a2

Browse files
author
Jim Ryan
committed
add configmap validation events
1 parent 8703f0e commit df087a2

File tree

4 files changed

+70
-26
lines changed

4 files changed

+70
-26
lines changed

‎cmd/nginx-ingress/main.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -151,7 +151,7 @@ func main() {
151151
mustProcessGlobalConfiguration(ctx)
152152

153153
cfgParams := configs.NewDefaultConfigParams(ctx, *nginxPlus)
154-
cfgParams = processConfigMaps(kubeClient, cfgParams, nginxManager, templateExecutor)
154+
cfgParams = processConfigMaps(kubeClient, cfgParams, nginxManager, templateExecutor, eventRecorder)
155155

156156
staticCfgParams := &configs.StaticConfigParams{
157157
DisableIPV6: *disableIPV6,
@@ -854,7 +854,7 @@ func mustProcessGlobalConfiguration(ctx context.Context) {
854854
}
855855
}
856856

857-
func processConfigMaps(kubeClient *kubernetes.Clientset, cfgParams *configs.ConfigParams, nginxManager nginx.Manager, templateExecutor *version1.TemplateExecutor) *configs.ConfigParams {
857+
func processConfigMaps(kubeClient *kubernetes.Clientset, cfgParams *configs.ConfigParams, nginxManager nginx.Manager, templateExecutor *version1.TemplateExecutor, eventLog record.EventRecorder) *configs.ConfigParams {
858858
l := nl.LoggerFromContext(cfgParams.Context)
859859
if *nginxConfigMaps != "" {
860860
ns, name, err := k8s.ParseNamespaceName(*nginxConfigMaps)
@@ -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)
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: 55 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -2,27 +2,40 @@ package configs
22

33
import (
44
"context"
5+
"fmt"
56
"strings"
67

78
v1 "k8s.io/api/core/v1"
9+
"k8s.io/client-go/tools/record"
810

911
"github.com/nginxinc/kubernetes-ingress/internal/configs/version1"
1012
nl "github.com/nginxinc/kubernetes-ingress/internal/logger"
1113
)
1214

15+
const (
16+
minimumInterval = 60
17+
invalidValueReason = "InvalidValue"
18+
parseErrorReason = "ParseError"
19+
)
20+
1321
// ParseConfigMap parses ConfigMap into ConfigParams.
1422
//
1523
//nolint:gocyclo
16-
func ParseConfigMap(ctx context.Context, cfgm *v1.ConfigMap, nginxPlus bool, hasAppProtect bool, hasAppProtectDos bool, hasTLSPassthrough bool) *ConfigParams {
24+
func ParseConfigMap(ctx context.Context, cfgm *v1.ConfigMap, nginxPlus bool, hasAppProtect bool, hasAppProtectDos bool, hasTLSPassthrough bool, eventLog record.EventRecorder) *ConfigParams {
1725
l := nl.LoggerFromContext(ctx)
1826
cfgParams := NewDefaultConfigParams(ctx, nginxPlus)
1927

28+
// valid values for server token are on | off | build | string;
29+
// oss can only use on | off
2030
if serverTokens, exists, err := GetMapKeyAsBool(cfgm.Data, "server-tokens", cfgm); exists {
31+
// this may be a build | string
2132
if err != nil {
2233
if nginxPlus {
2334
cfgParams.ServerTokens = cfgm.Data["server-tokens"]
2435
} else {
25-
nl.Error(l, err)
36+
errorText := fmt.Sprintf("Configmap %s/%s: server-tokens key must be a bool for oss", cfgm.GetNamespace(), cfgm.GetName())
37+
nl.Error(l, errorText)
38+
eventLog.Event(cfgm, v1.EventTypeWarning, invalidValueReason, errorText)
2639
}
2740
} else {
2841
cfgParams.ServerTokens = "off"
@@ -35,13 +48,17 @@ func ParseConfigMap(ctx context.Context, cfgm *v1.ConfigMap, nginxPlus bool, has
3548
if lbMethod, exists := cfgm.Data["lb-method"]; exists {
3649
if nginxPlus {
3750
if parsedMethod, err := ParseLBMethodForPlus(lbMethod); err != nil {
38-
nl.Errorf(l, "Configmap %s/%s: Invalid value for the lb-method key: got %q: %v", cfgm.GetNamespace(), cfgm.GetName(), lbMethod, err)
51+
errorText := fmt.Sprintf("Configmap %s/%s: Invalid value for the lb-method key: got %q: %v", cfgm.GetNamespace(), cfgm.GetName(), lbMethod, err)
52+
nl.Error(l, errorText)
53+
eventLog.Event(cfgm, v1.EventTypeWarning, invalidValueReason, errorText)
3954
} else {
4055
cfgParams.LBMethod = parsedMethod
4156
}
4257
} else {
4358
if parsedMethod, err := ParseLBMethod(lbMethod); err != nil {
44-
nl.Errorf(l, "Configmap %s/%s: Invalid value for the lb-method key: got %q: %v", cfgm.GetNamespace(), cfgm.GetName(), lbMethod, err)
59+
errorText := fmt.Sprintf("Configmap %s/%s: Invalid value for the lb-method key: got %q: %v", cfgm.GetNamespace(), cfgm.GetName(), lbMethod, err)
60+
nl.Error(l, errorText)
61+
eventLog.Event(cfgm, v1.EventTypeWarning, invalidValueReason, errorText)
4562
} else {
4663
cfgParams.LBMethod = parsedMethod
4764
}
@@ -90,52 +107,68 @@ func ParseConfigMap(ctx context.Context, cfgm *v1.ConfigMap, nginxPlus bool, has
90107

91108
if HTTP2, exists, err := GetMapKeyAsBool(cfgm.Data, "http2", cfgm); exists {
92109
if err != nil {
93-
nl.Error(l, err)
110+
errorText := fmt.Sprintf("Configmap %s/%s: Invalid value for the http key: got %t: %v", cfgm.GetNamespace(), cfgm.GetName(), HTTP2, err)
111+
nl.Error(l, errorText)
112+
eventLog.Event(cfgm, v1.EventTypeWarning, invalidValueReason, errorText)
94113
} else {
95114
cfgParams.HTTP2 = HTTP2
96115
}
97116
}
98117

99118
if redirectToHTTPS, exists, err := GetMapKeyAsBool(cfgm.Data, "redirect-to-https", cfgm); exists {
100119
if err != nil {
101-
nl.Error(l, err)
120+
errorText := fmt.Sprintf("Configmap %s/%s: Invalid value for the redirect-to-https key: got %t: %v", cfgm.GetNamespace(), cfgm.GetName(), redirectToHTTPS, err)
121+
nl.Error(l, errorText)
122+
eventLog.Event(cfgm, v1.EventTypeWarning, invalidValueReason, errorText)
102123
} else {
103124
cfgParams.RedirectToHTTPS = redirectToHTTPS
104125
}
105126
}
106127

107128
if sslRedirect, exists, err := GetMapKeyAsBool(cfgm.Data, "ssl-redirect", cfgm); exists {
108129
if err != nil {
109-
nl.Error(l, err)
130+
errorText := fmt.Sprintf("Configmap %s/%s: Invalid value for the ssl-redirect key: got %t: %v", cfgm.GetNamespace(), cfgm.GetName(), sslRedirect, err)
131+
nl.Error(l, errorText)
132+
eventLog.Event(cfgm, v1.EventTypeWarning, invalidValueReason, errorText)
110133
} else {
111134
cfgParams.SSLRedirect = sslRedirect
112135
}
113136
}
114137

115138
if hsts, exists, err := GetMapKeyAsBool(cfgm.Data, "hsts", cfgm); exists {
116139
if err != nil {
117-
nl.Error(l, err)
140+
errorText := fmt.Sprintf("Configmap %s/%s: Invalid value for the hsts key: got %t: %v", cfgm.GetNamespace(), cfgm.GetName(), hsts, err)
141+
nl.Error(l, errorText)
142+
eventLog.Event(cfgm, v1.EventTypeWarning, invalidValueReason, errorText)
118143
} else {
119144
parsingErrors := false
120145

121146
hstsMaxAge, existsMA, err := GetMapKeyAsInt64(cfgm.Data, "hsts-max-age", cfgm)
122147
if existsMA && err != nil {
123-
nl.Error(l, err)
148+
errorText := fmt.Sprintf("Configmap %s/%s: Invalid value for the hsts-max-age key: got %d: %v", cfgm.GetNamespace(), cfgm.GetName(), hstsMaxAge, err)
149+
nl.Error(l, errorText)
150+
eventLog.Event(cfgm, v1.EventTypeWarning, invalidValueReason, errorText)
124151
parsingErrors = true
125152
}
126153
hstsIncludeSubdomains, existsIS, err := GetMapKeyAsBool(cfgm.Data, "hsts-include-subdomains", cfgm)
127154
if existsIS && err != nil {
128-
nl.Error(l, err)
155+
errorText := fmt.Sprintf("Configmap %s/%s: Invalid value for the hsts-include-subdomains key: got %t: %v", cfgm.GetNamespace(), cfgm.GetName(), hstsIncludeSubdomains, err)
156+
nl.Error(l, errorText)
157+
eventLog.Event(cfgm, v1.EventTypeWarning, invalidValueReason, errorText)
129158
parsingErrors = true
130159
}
131160
hstsBehindProxy, existsBP, err := GetMapKeyAsBool(cfgm.Data, "hsts-behind-proxy", cfgm)
132161
if existsBP && err != nil {
133-
nl.Error(l, err)
162+
errorText := fmt.Sprintf("Configmap %s/%s: Invalid value for the hsts-behind-proxy key: got %t: %v", cfgm.GetNamespace(), cfgm.GetName(), hstsBehindProxy, err)
163+
nl.Error(l, errorText)
164+
eventLog.Event(cfgm, v1.EventTypeWarning, invalidValueReason, errorText)
134165
parsingErrors = true
135166
}
136167

137168
if parsingErrors {
138-
nl.Errorf(l, "Configmap %s/%s: There are configuration issues with hsts annotations, skipping options for all hsts settings", cfgm.GetNamespace(), cfgm.GetName())
169+
errorText := fmt.Sprintf("Configmap %s/%s: There are configuration issues with hsts annotations, skipping options for all hsts settings", cfgm.GetNamespace(), cfgm.GetName())
170+
nl.Error(l, errorText)
171+
eventLog.Event(cfgm, v1.EventTypeWarning, parseErrorReason, errorText)
139172
} else {
140173
cfgParams.HSTS = hsts
141174
if existsMA {
@@ -154,18 +187,22 @@ func ParseConfigMap(ctx context.Context, cfgm *v1.ConfigMap, nginxPlus bool, has
154187
if proxyProtocol, exists, err := GetMapKeyAsBool(cfgm.Data, "proxy-protocol", cfgm); exists {
155188
if err != nil {
156189
nl.Error(l, err)
190+
errorText := fmt.Sprintf("Configmap %s/%s: Invalid value for the proxy-protocol key: got %t: %v", cfgm.GetNamespace(), cfgm.GetName(), proxyProtocol, err)
191+
nl.Error(l, errorText)
192+
eventLog.Event(cfgm, v1.EventTypeWarning, invalidValueReason, errorText)
157193
} else {
158194
cfgParams.ProxyProtocol = proxyProtocol
159195
}
160196
}
161197

162198
if realIPHeader, exists := cfgm.Data["real-ip-header"]; exists {
163199
if hasTLSPassthrough {
164-
msg := "Configmap %s/%s: key real-ip-header is ignored, directive real_ip_header is automatically set to 'proxy_protocol' when TLS passthrough is enabled."
200+
errorText := fmt.Sprintf("Configmap %s/%s: key real-ip-header is ignored, directive real_ip_header is automatically set to 'proxy_protocol' when TLS passthrough is enabled.", cfgm.GetNamespace(), cfgm.GetName())
165201
if realIPHeader == "proxy_protocol" {
166-
nl.Infof(l, msg, cfgm.GetNamespace(), cfgm.GetName())
202+
nl.Info(l, errorText)
167203
} else {
168-
nl.Errorf(l, msg, cfgm.GetNamespace(), cfgm.GetName())
204+
nl.Error(l, errorText)
205+
eventLog.Event(cfgm, v1.EventTypeWarning, invalidValueReason, errorText)
169206
}
170207
} else {
171208
cfgParams.RealIPHeader = realIPHeader
@@ -178,7 +215,9 @@ func ParseConfigMap(ctx context.Context, cfgm *v1.ConfigMap, nginxPlus bool, has
178215

179216
if realIPRecursive, exists, err := GetMapKeyAsBool(cfgm.Data, "real-ip-recursive", cfgm); exists {
180217
if err != nil {
181-
nl.Error(l, err)
218+
errorText := fmt.Sprintf("Configmap %s/%s: Invalid value for the real-ip-recursive key: got %t: %v", cfgm.GetNamespace(), cfgm.GetName(), realIPRecursive, err)
219+
nl.Error(l, errorText)
220+
eventLog.Event(cfgm, v1.EventTypeWarning, invalidValueReason, errorText)
182221
} else {
183222
cfgParams.RealIPRecursive = realIPRecursive
184223
}

‎internal/configs/configmaps_test.go‎

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import (
55
"testing"
66

77
v1 "k8s.io/api/core/v1"
8+
"k8s.io/client-go/tools/record"
89
)
910

1011
func TestParseConfigMapWithAppProtectCompressedRequestsAction(t *testing.T) {
@@ -45,7 +46,7 @@ func TestParseConfigMapWithAppProtectCompressedRequestsAction(t *testing.T) {
4546
"app-protect-compressed-requests-action": test.action,
4647
},
4748
}
48-
result := ParseConfigMap(context.Background(), cm, nginxPlus, hasAppProtect, hasAppProtectDos, hasTLSPassthrough)
49+
result := ParseConfigMap(context.Background(), cm, nginxPlus, hasAppProtect, hasAppProtectDos, hasTLSPassthrough, makeEventLogger())
4950
if result.MainAppProtectCompressedRequestsAction != test.expect {
5051
t.Errorf("ParseConfigMap() returned %q but expected %q for the case %s", result.MainAppProtectCompressedRequestsAction, test.expect, test.msg)
5152
}
@@ -114,7 +115,7 @@ func TestParseConfigMapWithAppProtectReconnectPeriod(t *testing.T) {
114115
"app-protect-reconnect-period-seconds": test.period,
115116
},
116117
}
117-
result := ParseConfigMap(context.Background(), cm, nginxPlus, hasAppProtect, hasAppProtectDos, hasTLSPassthrough)
118+
result := ParseConfigMap(context.Background(), cm, nginxPlus, hasAppProtect, hasAppProtectDos, hasTLSPassthrough, makeEventLogger())
118119
if result.MainAppProtectReconnectPeriod != test.expect {
119120
t.Errorf("ParseConfigMap() returned %q but expected %q for the case %s", result.MainAppProtectReconnectPeriod, test.expect, test.msg)
120121
}
@@ -155,7 +156,7 @@ func TestParseConfigMapWithTLSPassthroughProxyProtocol(t *testing.T) {
155156
"real-ip-header": test.realIPheader,
156157
},
157158
}
158-
result := ParseConfigMap(context.Background(), cm, nginxPlus, hasAppProtect, hasAppProtectDos, hasTLSPassthrough)
159+
result := ParseConfigMap(context.Background(), cm, nginxPlus, hasAppProtect, hasAppProtectDos, hasTLSPassthrough, makeEventLogger())
159160
if result.RealIPHeader != test.want {
160161
t.Errorf("want %q, got %q", test.want, result.RealIPHeader)
161162
}
@@ -197,7 +198,7 @@ func TestParseConfigMapWithoutTLSPassthroughProxyProtocol(t *testing.T) {
197198
"real-ip-header": test.realIPheader,
198199
},
199200
}
200-
result := ParseConfigMap(context.Background(), cm, nginxPlus, hasAppProtect, hasAppProtectDos, hasTLSPassthrough)
201+
result := ParseConfigMap(context.Background(), cm, nginxPlus, hasAppProtect, hasAppProtectDos, hasTLSPassthrough, makeEventLogger())
201202
if result.RealIPHeader != test.want {
202203
t.Errorf("want %q, got %q", test.want, result.RealIPHeader)
203204
}
@@ -244,7 +245,7 @@ func TestParseConfigMapAccessLog(t *testing.T) {
244245
"access-log-off": test.accessLogOff,
245246
},
246247
}
247-
result := ParseConfigMap(context.Background(), cm, nginxPlus, hasAppProtect, hasAppProtectDos, hasTLSPassthrough)
248+
result := ParseConfigMap(context.Background(), cm, nginxPlus, hasAppProtect, hasAppProtectDos, hasTLSPassthrough, makeEventLogger())
248249
if result.MainAccessLog != test.want {
249250
t.Errorf("want %q, got %q", test.want, result.MainAccessLog)
250251
}
@@ -276,10 +277,14 @@ func TestParseConfigMapAccessLogDefault(t *testing.T) {
276277
"access-log-off": "False",
277278
},
278279
}
279-
result := ParseConfigMap(context.Background(), cm, nginxPlus, hasAppProtect, hasAppProtectDos, hasTLSPassthrough)
280+
result := ParseConfigMap(context.Background(), cm, nginxPlus, hasAppProtect, hasAppProtectDos, hasTLSPassthrough, makeEventLogger())
280281
if result.MainAccessLog != test.want {
281282
t.Errorf("want %q, got %q", test.want, result.MainAccessLog)
282283
}
283284
})
284285
}
285286
}
287+
288+
func makeEventLogger() record.EventRecorder {
289+
return record.NewFakeRecorder(1024)
290+
}

‎internal/k8s/controller.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -843,7 +843,7 @@ func (lbc *LoadBalancerController) updateAllConfigs() {
843843
cfgParams := configs.NewDefaultConfigParams(ctx, lbc.isNginxPlus)
844844

845845
if lbc.configMap != nil {
846-
cfgParams = configs.ParseConfigMap(ctx, lbc.configMap, lbc.isNginxPlus, lbc.appProtectEnabled, lbc.appProtectDosEnabled, lbc.configuration.isTLSPassthroughEnabled)
846+
cfgParams = configs.ParseConfigMap(ctx, lbc.configMap, lbc.isNginxPlus, lbc.appProtectEnabled, lbc.appProtectDosEnabled, lbc.configuration.isTLSPassthroughEnabled, lbc.recorder)
847847
}
848848

849849
resources := lbc.configuration.GetResources()

0 commit comments

Comments
 (0)