Skip to content

Commit e04179f

Browse files
john-david3john-david3
authored
Add config parser tests for SSLCerts (#1050)
1 parent 63c3abb commit e04179f

File tree

5 files changed

+262
-31
lines changed

5 files changed

+262
-31
lines changed

‎internal/watcher/instance/nginx_config_parser_test.go

Lines changed: 106 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,11 @@ import (
1212
"net/http"
1313
"net/http/httptest"
1414
"os"
15+
"sort"
1516
"testing"
1617

18+
"google.golang.org/protobuf/proto"
19+
1720
"google.golang.org/protobuf/types/known/timestamppb"
1821

1922
"github.com/nginx/agent/v3/internal/model"
@@ -260,6 +263,7 @@ var (
260263
`
261264
)
262265

266+
// nolint: maintidx
263267
func TestNginxConfigParser_Parse(t *testing.T) {
264268
ctx := context.Background()
265269
dir := t.TempDir()
@@ -287,6 +291,23 @@ func TestNginxConfigParser_Parse(t *testing.T) {
287291
defer helpers.RemoveFileWithErrorCheck(t, allowedFile.Name())
288292
fileMetaAllowedFiles, err := files.FileMeta(allowedFile.Name())
289293
require.NoError(t, err)
294+
allowedFileWithMetas := mpi.File{FileMeta: fileMetaAllowedFiles}
295+
296+
_, cert := helpers.GenerateSelfSignedCert(t)
297+
certContents := helpers.Cert{Name: "nginx.cert", Type: "CERTIFICATE", Contents: cert}
298+
certFile := helpers.WriteCertFiles(t, dir, certContents)
299+
require.NotNil(t, certFile)
300+
fileMetaCertFiles, err := files.FileMetaWithCertificate(certFile)
301+
require.NoError(t, err)
302+
certFileWithMetas := mpi.File{FileMeta: fileMetaCertFiles}
303+
304+
_, diffCert := helpers.GenerateSelfSignedCert(t)
305+
diffCertContents := helpers.Cert{Name: "nginx1.cert", Type: "CERTIFICATE", Contents: diffCert}
306+
diffCertFile := helpers.WriteCertFiles(t, dir, diffCertContents)
307+
require.NotNil(t, diffCertFile)
308+
diffFileMetaCertFiles, err := files.FileMetaWithCertificate(diffCertFile)
309+
require.NoError(t, err)
310+
diffCertFileWithMetas := mpi.File{FileMeta: diffFileMetaCertFiles}
290311

291312
tests := []struct {
292313
instance *mpi.Instance
@@ -341,34 +362,13 @@ func TestNginxConfigParser_Parse(t *testing.T) {
341362
instance: protos.GetNginxPlusInstance([]string{}),
342363
content: testconfig.GetNginxConfigWithNotAllowedDir(errorLog.Name(), allowedFile.Name(),
343364
notAllowedFile.Name(), accessLog.Name()),
344-
expectedConfigContext: &model.NginxConfigContext{
345-
StubStatus: &model.APIDetails{},
346-
PlusAPI: &model.APIDetails{},
347-
InstanceID: protos.GetNginxPlusInstance([]string{}).GetInstanceMeta().GetInstanceId(),
348-
Files: []*mpi.File{
349-
{
350-
FileMeta: fileMetaAllowedFiles,
351-
},
352-
},
353-
AccessLogs: []*model.AccessLog{
354-
{
355-
Name: accessLog.Name(),
356-
Format: "$remote_addr - $remote_user [$time_local] \"$request\" $status $body_bytes_sent " +
357-
"\"$http_referer\" \"$http_user_agent\" \"$http_x_forwarded_for\" \"$bytes_sent\" " +
358-
"\"$request_length\" \"$request_time\" \"$gzip_ratio\" $server_protocol ",
359-
Permissions: "0600",
360-
Readable: true,
361-
},
362-
},
363-
ErrorLogs: []*model.ErrorLog{
364-
{
365-
Name: errorLog.Name(),
366-
Permissions: "0600",
367-
Readable: true,
368-
},
369-
},
370-
NAPSysLogServers: nil,
371-
},
365+
expectedConfigContext: modelHelpers.GetConfigContextWithFiles(
366+
accessLog.Name(),
367+
errorLog.Name(),
368+
[]*mpi.File{&allowedFileWithMetas},
369+
protos.GetNginxPlusInstance([]string{}).GetInstanceMeta().GetInstanceId(),
370+
nil,
371+
),
372372
expectedLog: "",
373373
allowedDirectories: []string{dir},
374374
},
@@ -427,6 +427,59 @@ func TestNginxConfigParser_Parse(t *testing.T) {
427427
"config; log errors to file to enable error monitoring",
428428
allowedDirectories: []string{dir},
429429
},
430+
{
431+
name: "Test 7: Check Parser for SSL Certs",
432+
instance: protos.GetNginxPlusInstance([]string{}),
433+
content: testconfig.GetNginxConfigWithSSLCerts(
434+
errorLog.Name(),
435+
accessLog.Name(),
436+
certFile,
437+
),
438+
expectedConfigContext: modelHelpers.GetConfigContextWithFiles(
439+
accessLog.Name(),
440+
errorLog.Name(),
441+
[]*mpi.File{&certFileWithMetas},
442+
protos.GetNginxPlusInstance([]string{}).GetInstanceMeta().GetInstanceId(),
443+
nil,
444+
),
445+
allowedDirectories: []string{dir},
446+
},
447+
{
448+
name: "Test 8: Check for multiple different SSL Certs",
449+
instance: protos.GetNginxPlusInstance([]string{}),
450+
content: testconfig.GetNginxConfigWithMultipleSSLCerts(
451+
errorLog.Name(),
452+
accessLog.Name(),
453+
certFile,
454+
diffCertFile,
455+
),
456+
expectedConfigContext: modelHelpers.GetConfigContextWithFiles(
457+
accessLog.Name(),
458+
errorLog.Name(),
459+
[]*mpi.File{&diffCertFileWithMetas, &certFileWithMetas},
460+
protos.GetNginxPlusInstance([]string{}).GetInstanceMeta().GetInstanceId(),
461+
nil,
462+
),
463+
allowedDirectories: []string{dir},
464+
},
465+
{
466+
name: "Test 9: Check for multiple same SSL Certs",
467+
instance: protos.GetNginxPlusInstance([]string{}),
468+
content: testconfig.GetNginxConfigWithMultipleSSLCerts(
469+
errorLog.Name(),
470+
accessLog.Name(),
471+
certFile,
472+
certFile,
473+
),
474+
expectedConfigContext: modelHelpers.GetConfigContextWithFiles(
475+
accessLog.Name(),
476+
errorLog.Name(),
477+
[]*mpi.File{&certFileWithMetas},
478+
protos.GetNginxPlusInstance([]string{}).GetInstanceMeta().GetInstanceId(),
479+
nil,
480+
),
481+
allowedDirectories: []string{dir},
482+
},
430483
}
431484

432485
for _, test := range tests {
@@ -455,16 +508,28 @@ func TestNginxConfigParser_Parse(t *testing.T) {
455508
require.NoError(t, parseError)
456509

457510
helpers.ValidateLog(t, test.expectedLog, logBuf)
458-
459511
logBuf.Reset()
460512

461-
assert.ElementsMatch(t, test.expectedConfigContext.Files, result.Files)
513+
sort.Slice(test.expectedConfigContext.Files, func(i, j int) bool {
514+
return test.expectedConfigContext.Files[i].GetFileMeta().GetName() >
515+
test.expectedConfigContext.Files[j].GetFileMeta().GetName()
516+
})
517+
518+
sort.Slice(result.Files, func(i, j int) bool {
519+
return result.Files[i].GetFileMeta().GetName() >
520+
result.Files[j].GetFileMeta().GetName()
521+
})
522+
523+
assert.Truef(t,
524+
protoListEqual(test.expectedConfigContext.Files, result.Files),
525+
"Expect %s Got %s", test.expectedConfigContext.Files, result.Files)
462526
assert.Equal(t, test.expectedConfigContext.NAPSysLogServers, result.NAPSysLogServers)
463527
assert.Equal(t, test.expectedConfigContext.PlusAPI, result.PlusAPI)
464528
assert.ElementsMatch(t, test.expectedConfigContext.AccessLogs, result.AccessLogs)
465529
assert.ElementsMatch(t, test.expectedConfigContext.ErrorLogs, result.ErrorLogs)
466530
assert.Equal(t, test.expectedConfigContext.StubStatus, result.StubStatus)
467531
assert.Equal(t, test.expectedConfigContext.InstanceID, result.InstanceID)
532+
assert.Equal(t, len(test.expectedConfigContext.Files), len(result.Files))
468533
})
469534
}
470535
}
@@ -1160,3 +1225,14 @@ func TestNginxConfigParser_checkDuplicate(t *testing.T) {
11601225
})
11611226
}
11621227
}
1228+
1229+
func protoListEqual(protoListA, protoListB []*mpi.File) bool {
1230+
for i := 0; i < len(protoListA); i++ {
1231+
res := proto.Equal(protoListA[i], protoListB[i])
1232+
if !res {
1233+
return false
1234+
}
1235+
}
1236+
1237+
return true
1238+
}

‎test/config/nginx/nginx-with-multiple-ssl-certs.conf

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
worker_processes 1;
2+
error_log %s;
3+
events {
4+
worker_connections 1024;
5+
}
6+
7+
http {
8+
default_type application/octet-stream;
9+
10+
log_format main '$remote_addr - $remote_user [$time_local] "$request" '
11+
'$status $body_bytes_sent "$http_referer" '
12+
'"$http_user_agent" "$http_x_forwarded_for" '
13+
'"$bytes_sent" "$request_length" "$request_time" '
14+
'"$gzip_ratio" $server_protocol ';
15+
16+
access_log %s main;
17+
18+
sendfile on;
19+
keepalive_timeout 65;
20+
21+
server {
22+
listen 8080;
23+
server_name localhost;
24+
25+
location / {
26+
root /usr/share/nginx/html;
27+
index index.html index.htm;
28+
}
29+
30+
ssl_certificate %s;
31+
ssl_certificate %s;
32+
33+
##
34+
# Enable Metrics
35+
##
36+
location /api {
37+
stub_status;
38+
allow 127.0.0.1;
39+
deny all;
40+
}
41+
42+
# redirect server error pages to the static page /50x.html
43+
#
44+
error_page 500 502 503 504 /50x.html;
45+
location = /50x.html {
46+
root /usr/share/nginx/html;
47+
}
48+
}
49+
}

‎test/config/nginx/nginx-with-ssl-certs.conf

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
worker_processes 1;
2+
error_log %s;
3+
events {
4+
worker_connections 1024;
5+
}
6+
7+
http {
8+
default_type application/octet-stream;
9+
10+
log_format main '$remote_addr - $remote_user [$time_local] "$request" '
11+
'$status $body_bytes_sent "$http_referer" '
12+
'"$http_user_agent" "$http_x_forwarded_for" '
13+
'"$bytes_sent" "$request_length" "$request_time" '
14+
'"$gzip_ratio" $server_protocol ';
15+
16+
access_log %s main;
17+
18+
sendfile on;
19+
keepalive_timeout 65;
20+
21+
server {
22+
listen 8080;
23+
server_name localhost;
24+
25+
location / {
26+
root /usr/share/nginx/html;
27+
index index.html index.htm;
28+
}
29+
30+
ssl_certificate %s;
31+
32+
##
33+
# Enable Metrics
34+
##
35+
location /api {
36+
stub_status;
37+
allow 127.0.0.1;
38+
deny all;
39+
}
40+
41+
# redirect server error pages to the static page /50x.html
42+
#
43+
error_page 500 502 503 504 /50x.html;
44+
location = /50x.html {
45+
root /usr/share/nginx/html;
46+
}
47+
}
48+
}

‎test/config/nginx_config.go

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,12 @@ var embedNginxConfWithMultipleAccessLogs string
1616
//go:embed nginx/nginx-not-allowed-dir.conf
1717
var embedNginxConfWithNotAllowedDir string
1818

19+
//go:embed nginx/nginx-with-ssl-certs.conf
20+
var embedNginxConfWithSSLCerts string
21+
22+
//go:embed nginx/nginx-with-multiple-ssl-certs.conf
23+
var embedNginxConfWithMultipleSSLCerts string
24+
1925
//go:embed nginx/nginx-ssl-certs-with-variables.conf
2026
var embedNginxConfWithSSLCertsWithVariables string
2127

@@ -41,3 +47,11 @@ func GetNginxConfigWithNotAllowedDir(errorLogFile, notAllowedFile, allowedFileDi
4147
func GetNginxConfWithSSLCertsWithVariables() string {
4248
return embedNginxConfWithSSLCertsWithVariables
4349
}
50+
51+
func GetNginxConfigWithSSLCerts(errorLogFile, accessLogFile, certFile string) string {
52+
return fmt.Sprintf(embedNginxConfWithSSLCerts, errorLogFile, accessLogFile, certFile)
53+
}
54+
55+
func GetNginxConfigWithMultipleSSLCerts(errorLogFile, accessLogFile, certFile1, certFile2 string) string {
56+
return fmt.Sprintf(embedNginxConfWithMultipleSSLCerts, errorLogFile, accessLogFile, certFile1, certFile2)
57+
}

‎test/model/config.go

Lines changed: 45 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,10 @@
55

66
package model
77

8-
import "github.com/nginx/agent/v3/internal/model"
8+
import (
9+
mpi "github.com/nginx/agent/v3/api/grpc/mpi/v1"
10+
"github.com/nginx/agent/v3/internal/model"
11+
)
912

1013
func GetConfigContext() *model.NginxConfigContext {
1114
return &model.NginxConfigContext{
@@ -116,3 +119,44 @@ func GetConfigContextWithoutErrorLog(
116119
NAPSysLogServers: syslogServers,
117120
}
118121
}
122+
123+
func GetConfigContextWithFiles(
124+
accessLogName,
125+
errorLogName string,
126+
files []*mpi.File,
127+
instanceID string,
128+
syslogServers []string,
129+
) *model.NginxConfigContext {
130+
return &model.NginxConfigContext{
131+
StubStatus: &model.APIDetails{
132+
URL: "",
133+
Listen: "",
134+
Location: "",
135+
},
136+
PlusAPI: &model.APIDetails{
137+
URL: "",
138+
Listen: "",
139+
Location: "",
140+
},
141+
Files: files,
142+
AccessLogs: []*model.AccessLog{
143+
{
144+
Name: accessLogName,
145+
Format: "$remote_addr - $remote_user [$time_local] \"$request\" $status $body_bytes_sent " +
146+
"\"$http_referer\" \"$http_user_agent\" \"$http_x_forwarded_for\" \"$bytes_sent\" " +
147+
"\"$request_length\" \"$request_time\" \"$gzip_ratio\" $server_protocol ",
148+
Readable: true,
149+
Permissions: "0600",
150+
},
151+
},
152+
ErrorLogs: []*model.ErrorLog{
153+
{
154+
Name: errorLogName,
155+
Readable: true,
156+
Permissions: "0600",
157+
},
158+
},
159+
InstanceID: instanceID,
160+
NAPSysLogServers: syslogServers,
161+
}
162+
}

0 commit comments

Comments
 (0)