Skip to content

Commit 2c1c724

Browse files
haywoodshhaywoodsh
committed
check nginxplus flag, add tests
Signed-off-by: Haywood Shannon <[email protected]>
1 parent 130dfc8 commit 2c1c724

File tree

5 files changed

+157
-86
lines changed

5 files changed

+157
-86
lines changed

‎build/Dockerfile‎

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -114,14 +114,12 @@ ENV NGINX_VERSION=${NGINX_PLUS_VERSION}
114114

115115
RUN --mount=type=secret,id=nginx-repo.crt,dst=/etc/apk/cert.pem,mode=0644 \
116116
--mount=type=secret,id=nginx-repo.key,dst=/etc/apk/cert.key,mode=0644 \
117-
--mount=type=bind,from=alpine-opentracing-lib,target=/tmp/ot/ \
118117
--mount=type=bind,from=nginx-files,src=nginx_signing.rsa.pub,target=/etc/apk/keys/nginx_signing.rsa.pub \
119118
--mount=type=bind,from=nginx-files,src=user_agent,target=/tmp/user_agent \
120119
--mount=type=bind,from=nginx-files,src=tracking.info,target=/tmp/nginx/reporting/tracking.info \
121120
export $(cat /tmp/user_agent) \
122121
&& printf "%s\n" "https://${PACKAGE_REPO}/plus/${NGINX_PLUS_VERSION}/alpine/v$(grep -E -o '^[0-9]+\.[0-9]+' /etc/alpine-release)/main" >> /etc/apk/repositories \
123122
&& apk add --no-cache nginx-plus nginx-plus-module-njs nginx-plus-module-fips-check libcap libcurl \
124-
&& cp -av /tmp/ot/usr/local/lib/libjaegertracing*so* /tmp/ot/usr/local/lib/libzipkin*so* /tmp/ot/usr/local/lib/libdd*so* /tmp/ot/usr/local/lib/libyaml*so* /usr/local/lib/ \
125123
&& mkdir -p /etc/nginx/reporting/ && cp -av /tmp/nginx/reporting/tracking.info /etc/nginx/reporting/tracking.info \
126124
&& ldconfig /usr/local/lib/ \
127125
&& sed -i -e '/nginx.com/d' /etc/apk/repositories
@@ -154,7 +152,6 @@ ENV NGINX_VERSION=${NGINX_PLUS_VERSION}
154152
RUN --mount=type=bind,from=alpine-fips-3.19,target=/tmp/fips/ \
155153
--mount=type=secret,id=nginx-repo.crt,dst=/etc/apk/cert.pem,mode=0644 \
156154
--mount=type=secret,id=nginx-repo.key,dst=/etc/apk/cert.key,mode=0644 \
157-
--mount=type=bind,from=alpine-opentracing-lib,target=/tmp/ot/ \
158155
--mount=type=bind,from=nginx-files,src=app-protect-security-updates.rsa.pub,target=/etc/apk/keys/app-protect-security-updates.rsa.pub \
159156
--mount=type=bind,from=nginx-files,src=nginx_signing.rsa.pub,target=/etc/apk/keys/nginx_signing.rsa.pub \
160157
--mount=type=bind,from=nginx-files,src=agent.sh,target=/usr/local/bin/agent.sh \
@@ -170,7 +167,6 @@ RUN --mount=type=bind,from=alpine-fips-3.19,target=/tmp/fips/ \
170167
&& cp -av /tmp/fips/usr/lib/ossl-modules/fips.so /usr/lib/ossl-modules/fips.so \
171168
&& cp -av /tmp/fips/usr/ssl/fipsmodule.cnf /usr/ssl/fipsmodule.cnf \
172169
&& cp -av /tmp/fips/etc/ssl/openssl.cnf /etc/ssl/openssl.cnf \
173-
&& cp -av /tmp/ot/usr/local/lib/libjaegertracing*so* /tmp/ot/usr/local/lib/libzipkin*so* /tmp/ot/usr/local/lib/libdd*so* /tmp/ot/usr/local/lib/libyaml*so* /usr/local/lib/ \
174170
&& mkdir -p /etc/nginx/reporting/ \
175171
&& cp -av /tmp/nginx/reporting/tracking.info /etc/nginx/reporting/tracking.info \
176172
&& ldconfig /usr/local/lib/ \
@@ -194,7 +190,6 @@ ENV NGINX_VERSION=${NGINX_PLUS_VERSION}
194190
RUN --mount=type=bind,from=alpine-fips-3.19,target=/tmp/fips/ \
195191
--mount=type=secret,id=nginx-repo.crt,dst=/etc/apk/cert.pem,mode=0644 \
196192
--mount=type=secret,id=nginx-repo.key,dst=/etc/apk/cert.key,mode=0644 \
197-
--mount=type=bind,from=alpine-opentracing-lib,target=/tmp/ot/ \
198193
--mount=type=bind,from=nginx-files,src=nginx_signing.rsa.pub,target=/etc/apk/keys/nginx_signing.rsa.pub \
199194
--mount=type=bind,from=nginx-files,src=agent.sh,target=/usr/local/bin/agent.sh \
200195
--mount=type=bind,from=nginx-files,src=nap-waf.sh,target=/usr/local/bin/nap-waf.sh \
@@ -208,7 +203,6 @@ RUN --mount=type=bind,from=alpine-fips-3.19,target=/tmp/fips/ \
208203
&& cp -av /tmp/fips/usr/lib/ossl-modules/fips.so /usr/lib/ossl-modules/fips.so \
209204
&& cp -av /tmp/fips/usr/ssl/fipsmodule.cnf /usr/ssl/fipsmodule.cnf \
210205
&& cp -av /tmp/fips/etc/ssl/openssl.cnf /etc/ssl/openssl.cnf \
211-
&& cp -av /tmp/ot/usr/local/lib/libjaegertracing*so* /tmp/ot/usr/local/lib/libzipkin*so* /tmp/ot/usr/local/lib/libdd*so* /tmp/ot/usr/local/lib/libyaml*so* /usr/local/lib/ \
212206
&& mkdir -p /etc/nginx/reporting/ \
213207
&& cp -av /tmp/nginx/reporting/tracking.info /etc/nginx/reporting/tracking.info \
214208
&& ldconfig /usr/local/lib/ \
@@ -229,7 +223,6 @@ ENV NGINX_VERSION=${NGINX_PLUS_VERSION}
229223
SHELL ["/bin/bash", "-o", "pipefail", "-c"]
230224
RUN --mount=type=secret,id=nginx-repo.crt,dst=/etc/ssl/nginx/nginx-repo.crt,mode=0644 \
231225
--mount=type=secret,id=nginx-repo.key,dst=/etc/ssl/nginx/nginx-repo.key,mode=0644 \
232-
--mount=type=bind,from=opentracing-lib,target=/tmp/ot/ \
233226
--mount=type=bind,from=nginx-files,src=nginx_signing.key,target=/tmp/nginx_signing.key \
234227
--mount=type=bind,from=nginx-files,src=app-protect-security-updates.key,target=/tmp/app-protect-security-updates.key \
235228
--mount=type=bind,from=nginx-files,src=90pkgs-nginx,target=/etc/apt/apt.conf.d/90pkgs-nginx \
@@ -245,7 +238,6 @@ RUN --mount=type=secret,id=nginx-repo.crt,dst=/etc/ssl/nginx/nginx-repo.crt,mode
245238
&& apt-get update \
246239
&& apt-get install --no-install-recommends --no-install-suggests -y nginx-plus nginx-plus-module-njs nginx-plus-module-fips-check \
247240
&& apt-get purge --auto-remove -y gpg \
248-
&& cp -av /tmp/ot/usr/local/lib/libjaegertracing*so* /tmp/ot/usr/local/lib/libzipkin*so* /tmp/ot/usr/local/lib/libdd*so* /tmp/ot/usr/local/lib/libyaml*so* /usr/local/lib/ \
249241
&& mkdir -p /etc/nginx/reporting/ \
250242
&& cp -av /tmp/nginx/reporting/tracking.info /etc/nginx/reporting/tracking.info \
251243
&& ldconfig \
@@ -262,7 +254,6 @@ ENV NGINX_VERSION=${NGINX_PLUS_VERSION}
262254

263255
RUN --mount=type=secret,id=nginx-repo.crt,dst=/etc/ssl/nginx/nginx-repo.crt,mode=0644 \
264256
--mount=type=secret,id=nginx-repo.key,dst=/etc/ssl/nginx/nginx-repo.key,mode=0644 \
265-
--mount=type=bind,from=opentracing-lib,target=/tmp/ot/ \
266257
--mount=type=bind,from=nginx-files,src=nginx_signing.key,target=/tmp/nginx_signing.key \
267258
--mount=type=bind,from=nginx-files,src=90pkgs-nginx,target=/etc/apt/apt.conf.d/90pkgs-nginx \
268259
--mount=type=bind,from=nginx-files,src=nap-waf-12.sources,target=/tmp/app-protect.sources \

‎internal/configs/configmaps.go‎

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -538,7 +538,7 @@ func ParseConfigMap(ctx context.Context, cfgm *v1.ConfigMap, nginxPlus bool, has
538538
cfgParams.MainOpenTracingTracerConfig = openTracingTracerConfig
539539
}
540540

541-
if cfgParams.MainOpenTracingTracer != "" || cfgParams.MainOpenTracingTracerConfig != "" {
541+
if cfgParams.MainOpenTracingTracer != "" && cfgParams.MainOpenTracingTracerConfig != "" {
542542
cfgParams.MainOpenTracingLoadModule = true
543543
}
544544

@@ -547,11 +547,14 @@ func ParseConfigMap(ctx context.Context, cfgm *v1.ConfigMap, nginxPlus bool, has
547547
nl.Error(l, err)
548548
eventLog.Event(cfgm, v1.EventTypeWarning, nl.EventReasonInvalidValue, err.Error())
549549
configOk = false
550+
} else if openTracing && nginxPlus {
551+
errorText := fmt.Sprintf("ConfigMap %s/%s key %s is not compatible with NGINX Plus", cfgm.Namespace, cfgm.Name, "opentracing")
552+
nl.Warn(l, errorText)
553+
eventLog.Event(cfgm, v1.EventTypeWarning, nl.EventReasonInvalidValue, errorText)
554+
configOk = false
555+
clearOpenTracingParams(cfgParams)
550556
} else if !openTracing {
551-
cfgParams.MainOpenTracingEnabled = false
552-
cfgParams.MainOpenTracingLoadModule = false
553-
cfgParams.MainOpenTracingTracer = ""
554-
cfgParams.MainOpenTracingTracerConfig = ""
557+
clearOpenTracingParams(cfgParams)
555558
} else {
556559
if cfgParams.MainOpenTracingLoadModule {
557560
cfgParams.MainOpenTracingEnabled = openTracing
@@ -674,6 +677,13 @@ func ParseConfigMap(ctx context.Context, cfgm *v1.ConfigMap, nginxPlus bool, has
674677
return cfgParams, configOk
675678
}
676679

680+
func clearOpenTracingParams(cfgParams *ConfigParams) {
681+
cfgParams.MainOpenTracingEnabled = false
682+
cfgParams.MainOpenTracingLoadModule = false
683+
cfgParams.MainOpenTracingTracer = ""
684+
cfgParams.MainOpenTracingTracerConfig = ""
685+
}
686+
677687
//nolint:gocyclo
678688
func parseConfigMapZoneSync(l *slog.Logger, cfgm *v1.ConfigMap, cfgParams *ConfigParams, eventLog record.EventRecorder, nginxPlus bool) (*ZoneSync, error) {
679689
if zoneSync, exists, err := GetMapKeyAsBool(cfgm.Data, "zone-sync", cfgm); exists {

‎internal/configs/configmaps_test.go‎

Lines changed: 142 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -1166,16 +1166,17 @@ func TestParseZoneSyncResolverIPV6MapResolverIPV6(t *testing.T) {
11661166
func makeEventLogger() record.EventRecorder {
11671167
return record.NewFakeRecorder(1024)
11681168
}
1169-
11701169
func TestOpenTracingConfiguration(t *testing.T) {
11711170
t.Parallel()
11721171
tests := []struct {
1173-
configMap *v1.ConfigMap
1174-
enabled bool
1175-
loadModule bool
1176-
tracer string
1177-
tracerConfig string
1178-
msg string
1172+
configMap *v1.ConfigMap
1173+
isPlus bool
1174+
expectedOpenTracingEnabled bool
1175+
expectedLoadModule bool
1176+
expectedTracer string
1177+
expectedTracerConfig string
1178+
expectedConfigOk bool
1179+
msg string
11791180
}{
11801181
{
11811182
configMap: &v1.ConfigMap{
@@ -1185,11 +1186,42 @@ func TestOpenTracingConfiguration(t *testing.T) {
11851186
"opentracing-tracer-config": "/etc/nginx/opentracing.json",
11861187
},
11871188
},
1188-
enabled: true,
1189-
loadModule: true,
1190-
tracer: "/usr/local/lib/libjaegertracing.so",
1191-
tracerConfig: "/etc/nginx/opentracing.json",
1192-
msg: "opentracing enabled",
1189+
isPlus: false,
1190+
expectedOpenTracingEnabled: true,
1191+
expectedLoadModule: true,
1192+
expectedTracer: "/usr/local/lib/libjaegertracing.so",
1193+
expectedTracerConfig: "/etc/nginx/opentracing.json",
1194+
expectedConfigOk: true,
1195+
msg: "oss: opentracing enabled (valid)",
1196+
},
1197+
{
1198+
configMap: &v1.ConfigMap{
1199+
Data: map[string]string{
1200+
"opentracing": "true",
1201+
"opentracing-tracer": "/usr/local/lib/libjaegertracing.so",
1202+
},
1203+
},
1204+
isPlus: false,
1205+
expectedOpenTracingEnabled: false,
1206+
expectedLoadModule: false,
1207+
expectedTracer: "/usr/local/lib/libjaegertracing.so",
1208+
expectedTracerConfig: "",
1209+
expectedConfigOk: false,
1210+
msg: "oss: opentracing enabled, tracer-config not set (invalid)",
1211+
},
1212+
{
1213+
configMap: &v1.ConfigMap{
1214+
Data: map[string]string{
1215+
"opentracing": "true",
1216+
},
1217+
},
1218+
isPlus: false,
1219+
expectedOpenTracingEnabled: false,
1220+
expectedLoadModule: false,
1221+
expectedTracer: "",
1222+
expectedTracerConfig: "",
1223+
expectedConfigOk: false,
1224+
msg: "oss: opentracing enabled, tracer and tracer-config not set (invalid)",
11931225
},
11941226
{
11951227
configMap: &v1.ConfigMap{
@@ -1199,56 +1231,132 @@ func TestOpenTracingConfiguration(t *testing.T) {
11991231
"opentracing-tracer-config": "/etc/nginx/opentracing.json",
12001232
},
12011233
},
1202-
enabled: false,
1203-
loadModule: false,
1204-
tracer: "",
1205-
tracerConfig: "",
1206-
msg: "opentracing disabled",
1234+
isPlus: false,
1235+
expectedOpenTracingEnabled: false,
1236+
expectedLoadModule: false,
1237+
expectedTracer: "",
1238+
expectedTracerConfig: "",
1239+
expectedConfigOk: true,
1240+
msg: "oss: opentracing disabled, tracer and tracer-config set (valid)",
12071241
},
12081242
{
12091243
configMap: &v1.ConfigMap{
12101244
Data: map[string]string{
12111245
"opentracing": "false",
12121246
},
12131247
},
1214-
enabled: false,
1215-
loadModule: false,
1216-
tracer: "",
1217-
tracerConfig: "",
1218-
msg: "opentracing disabled",
1248+
isPlus: false,
1249+
expectedOpenTracingEnabled: false,
1250+
expectedLoadModule: false,
1251+
expectedTracer: "",
1252+
expectedTracerConfig: "",
1253+
expectedConfigOk: true,
1254+
msg: "oss: opentracing disabled (valid)",
1255+
},
1256+
{
1257+
configMap: &v1.ConfigMap{
1258+
Data: map[string]string{
1259+
"opentracing": "false",
1260+
},
1261+
},
1262+
isPlus: true,
1263+
expectedOpenTracingEnabled: false,
1264+
expectedLoadModule: false,
1265+
expectedTracer: "",
1266+
expectedTracerConfig: "",
1267+
expectedConfigOk: true,
1268+
msg: "plus: opentracing explicitly disabled (valid)",
1269+
},
1270+
{
1271+
configMap: &v1.ConfigMap{
1272+
Data: map[string]string{},
1273+
},
1274+
isPlus: true,
1275+
expectedOpenTracingEnabled: false,
1276+
expectedLoadModule: false,
1277+
expectedTracer: "",
1278+
expectedTracerConfig: "",
1279+
expectedConfigOk: true,
1280+
msg: "plus: no opentracing keys set (valid)",
1281+
},
1282+
{
1283+
configMap: &v1.ConfigMap{
1284+
Data: map[string]string{
1285+
"opentracing": "false",
1286+
"opentracing-tracer": "/usr/local/lib/libjaegertracing.so",
1287+
"opentracing-tracer-config": "/etc/nginx/opentracing.json",
1288+
},
1289+
},
1290+
isPlus: true,
1291+
expectedOpenTracingEnabled: false,
1292+
expectedLoadModule: false,
1293+
expectedTracer: "",
1294+
expectedTracerConfig: "",
1295+
expectedConfigOk: true,
1296+
msg: "plus: opentracing disabled, tracer and tracer-config set (valid)",
1297+
},
1298+
{
1299+
configMap: &v1.ConfigMap{
1300+
Data: map[string]string{
1301+
"opentracing": "true",
1302+
},
1303+
},
1304+
isPlus: true,
1305+
expectedOpenTracingEnabled: false,
1306+
expectedLoadModule: false,
1307+
expectedTracer: "",
1308+
expectedTracerConfig: "",
1309+
expectedConfigOk: false,
1310+
msg: "plus: opentracing enabled (invalid)",
1311+
},
1312+
{
1313+
configMap: &v1.ConfigMap{
1314+
Data: map[string]string{
1315+
"opentracing": "true",
1316+
"opentracing-tracer": "/usr/local/lib/libjaegertracing.so",
1317+
"opentracing-tracer-config": "/etc/nginx/opentracing.json",
1318+
},
1319+
},
1320+
isPlus: true,
1321+
expectedOpenTracingEnabled: false,
1322+
expectedLoadModule: false,
1323+
expectedTracer: "",
1324+
expectedTracerConfig: "",
1325+
expectedConfigOk: false,
1326+
msg: "plus: opentracing enabled, tracer and tracer-config set (invalid)",
12191327
},
12201328
}
1221-
nginxPlus := false
1329+
12221330
hasAppProtect := false
12231331
hasAppProtectDos := false
12241332
hasTLSPassthrough := false
12251333

12261334
for _, test := range tests {
12271335
t.Run(test.msg, func(t *testing.T) {
1228-
result, configOk := ParseConfigMap(context.Background(), test.configMap, nginxPlus,
1336+
result, configOk := ParseConfigMap(context.Background(), test.configMap, test.isPlus,
12291337
hasAppProtect, hasAppProtectDos, hasTLSPassthrough, makeEventLogger())
12301338

1231-
if !configOk {
1232-
t.Errorf("Expected valid config, got invalid")
1339+
if configOk != test.expectedConfigOk {
1340+
t.Errorf("configOk: want %v, got %v", test.expectedConfigOk, configOk)
12331341
}
1234-
if result.MainOpenTracingEnabled != test.enabled {
1342+
if result.MainOpenTracingEnabled != test.expectedOpenTracingEnabled {
12351343
t.Errorf("MainOpenTracingEnabled: want %v, got %v",
1236-
test.enabled, result.MainOpenTracingEnabled)
1344+
test.expectedOpenTracingEnabled, result.MainOpenTracingEnabled)
12371345
}
12381346

1239-
if result.MainOpenTracingLoadModule != test.loadModule {
1347+
if result.MainOpenTracingLoadModule != test.expectedLoadModule {
12401348
t.Errorf("MainOpenTracingLoadModule: want %v, got %v",
1241-
test.loadModule, result.MainOpenTracingLoadModule)
1349+
test.expectedLoadModule, result.MainOpenTracingLoadModule)
12421350
}
12431351

1244-
if result.MainOpenTracingTracer != test.tracer {
1352+
if result.MainOpenTracingTracer != test.expectedTracer {
12451353
t.Errorf("MainOpenTracingTracer: want %q, got %q",
1246-
test.tracer, result.MainOpenTracingTracer)
1354+
test.expectedTracer, result.MainOpenTracingTracer)
12471355
}
12481356

1249-
if result.MainOpenTracingTracerConfig != test.tracerConfig {
1357+
if result.MainOpenTracingTracerConfig != test.expectedTracerConfig {
12501358
t.Errorf("MainOpenTracingTracerConfig: want %q, got %q",
1251-
test.tracerConfig, result.MainOpenTracingTracerConfig)
1359+
test.expectedTracerConfig, result.MainOpenTracingTracerConfig)
12521360
}
12531361
})
12541362
}

‎internal/nginx/version.go‎

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -99,12 +99,3 @@ func extractPlusVersionValues(input string) (int, int, error) {
9999

100100
return rValue, pValue, nil
101101
}
102-
103-
// IsOpenTracingSupported takes version and determines if the version
104-
// supports the Open Tracing lib.
105-
//
106-
// Support for OpenTracing is dropped in NGINX Plus R34.
107-
// Earlier NGINX Plus versions and NGINX OSS support OpenTracing.
108-
func IsOpenTracingSupported(version Version) bool {
109-
return !version.IsPlus
110-
}

‎internal/nginx/version_test.go‎

Lines changed: 0 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -127,35 +127,6 @@ func TestNginxVersionPlusGreaterThanOrEqualTo(t *testing.T) {
127127
}
128128
}
129129

130-
func TestIsOpenTracingSupported(t *testing.T) {
131-
t.Parallel()
132-
133-
tt := []struct {
134-
version nginx.Version
135-
want bool
136-
}{
137-
{
138-
version: nginx.NewVersion("nginx version: nginx/1.25.1"),
139-
want: true,
140-
},
141-
{
142-
version: nginx.NewVersion("nginx version: nginx/1.25.1 (nginx-plus-r34)"),
143-
want: false,
144-
},
145-
{
146-
version: nginx.NewVersion("nginx version: nginx/1.25.1 (nginx-plus-r34-p1)"),
147-
want: false,
148-
},
149-
}
150-
151-
for _, tc := range tt {
152-
got := nginx.IsOpenTracingSupported(tc.version)
153-
if tc.want != got {
154-
t.Errorf("%+v", tc.version)
155-
}
156-
}
157-
}
158-
159130
func TestNginxVersionPlusGreaterThanOrEqualToFailsOnInalidInput(t *testing.T) {
160131
t.Parallel()
161132

0 commit comments

Comments
 (0)