diff --git a/internal/watcher/instance/nginx_config_parser_test.go b/internal/watcher/instance/nginx_config_parser_test.go index 0159fc060..e0c22e9be 100644 --- a/internal/watcher/instance/nginx_config_parser_test.go +++ b/internal/watcher/instance/nginx_config_parser_test.go @@ -12,8 +12,11 @@ import ( "net/http" "net/http/httptest" "os" + "sort" "testing" + "google.golang.org/protobuf/proto" + "google.golang.org/protobuf/types/known/timestamppb" "github.com/nginx/agent/v3/internal/model" @@ -260,6 +263,7 @@ var ( ` ) +// nolint: maintidx func TestNginxConfigParser_Parse(t *testing.T) { ctx := context.Background() dir := t.TempDir() @@ -287,6 +291,23 @@ func TestNginxConfigParser_Parse(t *testing.T) { defer helpers.RemoveFileWithErrorCheck(t, allowedFile.Name()) fileMetaAllowedFiles, err := files.FileMeta(allowedFile.Name()) require.NoError(t, err) + allowedFileWithMetas := mpi.File{FileMeta: fileMetaAllowedFiles} + + _, cert := helpers.GenerateSelfSignedCert(t) + certContents := helpers.Cert{Name: "nginx.cert", Type: "CERTIFICATE", Contents: cert} + certFile := helpers.WriteCertFiles(t, dir, certContents) + require.NotNil(t, certFile) + fileMetaCertFiles, err := files.FileMetaWithCertificate(certFile) + require.NoError(t, err) + certFileWithMetas := mpi.File{FileMeta: fileMetaCertFiles} + + _, diffCert := helpers.GenerateSelfSignedCert(t) + diffCertContents := helpers.Cert{Name: "nginx1.cert", Type: "CERTIFICATE", Contents: diffCert} + diffCertFile := helpers.WriteCertFiles(t, dir, diffCertContents) + require.NotNil(t, diffCertFile) + diffFileMetaCertFiles, err := files.FileMetaWithCertificate(diffCertFile) + require.NoError(t, err) + diffCertFileWithMetas := mpi.File{FileMeta: diffFileMetaCertFiles} tests := []struct { instance *mpi.Instance @@ -341,34 +362,13 @@ func TestNginxConfigParser_Parse(t *testing.T) { instance: protos.GetNginxPlusInstance([]string{}), content: testconfig.GetNginxConfigWithNotAllowedDir(errorLog.Name(), allowedFile.Name(), notAllowedFile.Name(), accessLog.Name()), - expectedConfigContext: &model.NginxConfigContext{ - StubStatus: &model.APIDetails{}, - PlusAPI: &model.APIDetails{}, - InstanceID: protos.GetNginxPlusInstance([]string{}).GetInstanceMeta().GetInstanceId(), - Files: []*mpi.File{ - { - FileMeta: fileMetaAllowedFiles, - }, - }, - AccessLogs: []*model.AccessLog{ - { - Name: accessLog.Name(), - Format: "$remote_addr - $remote_user [$time_local] \"$request\" $status $body_bytes_sent " + - "\"$http_referer\" \"$http_user_agent\" \"$http_x_forwarded_for\" \"$bytes_sent\" " + - "\"$request_length\" \"$request_time\" \"$gzip_ratio\" $server_protocol ", - Permissions: "0600", - Readable: true, - }, - }, - ErrorLogs: []*model.ErrorLog{ - { - Name: errorLog.Name(), - Permissions: "0600", - Readable: true, - }, - }, - NAPSysLogServers: nil, - }, + expectedConfigContext: modelHelpers.GetConfigContextWithFiles( + accessLog.Name(), + errorLog.Name(), + []*mpi.File{&allowedFileWithMetas}, + protos.GetNginxPlusInstance([]string{}).GetInstanceMeta().GetInstanceId(), + nil, + ), expectedLog: "", allowedDirectories: []string{dir}, }, @@ -427,6 +427,59 @@ func TestNginxConfigParser_Parse(t *testing.T) { "config; log errors to file to enable error monitoring", allowedDirectories: []string{dir}, }, + { + name: "Test 7: Check Parser for SSL Certs", + instance: protos.GetNginxPlusInstance([]string{}), + content: testconfig.GetNginxConfigWithSSLCerts( + errorLog.Name(), + accessLog.Name(), + certFile, + ), + expectedConfigContext: modelHelpers.GetConfigContextWithFiles( + accessLog.Name(), + errorLog.Name(), + []*mpi.File{&certFileWithMetas}, + protos.GetNginxPlusInstance([]string{}).GetInstanceMeta().GetInstanceId(), + nil, + ), + allowedDirectories: []string{dir}, + }, + { + name: "Test 8: Check for multiple different SSL Certs", + instance: protos.GetNginxPlusInstance([]string{}), + content: testconfig.GetNginxConfigWithMultipleSSLCerts( + errorLog.Name(), + accessLog.Name(), + certFile, + diffCertFile, + ), + expectedConfigContext: modelHelpers.GetConfigContextWithFiles( + accessLog.Name(), + errorLog.Name(), + []*mpi.File{&diffCertFileWithMetas, &certFileWithMetas}, + protos.GetNginxPlusInstance([]string{}).GetInstanceMeta().GetInstanceId(), + nil, + ), + allowedDirectories: []string{dir}, + }, + { + name: "Test 9: Check for multiple same SSL Certs", + instance: protos.GetNginxPlusInstance([]string{}), + content: testconfig.GetNginxConfigWithMultipleSSLCerts( + errorLog.Name(), + accessLog.Name(), + certFile, + certFile, + ), + expectedConfigContext: modelHelpers.GetConfigContextWithFiles( + accessLog.Name(), + errorLog.Name(), + []*mpi.File{&certFileWithMetas}, + protos.GetNginxPlusInstance([]string{}).GetInstanceMeta().GetInstanceId(), + nil, + ), + allowedDirectories: []string{dir}, + }, } for _, test := range tests { @@ -455,16 +508,28 @@ func TestNginxConfigParser_Parse(t *testing.T) { require.NoError(t, parseError) helpers.ValidateLog(t, test.expectedLog, logBuf) - logBuf.Reset() - assert.ElementsMatch(t, test.expectedConfigContext.Files, result.Files) + sort.Slice(test.expectedConfigContext.Files, func(i, j int) bool { + return test.expectedConfigContext.Files[i].GetFileMeta().GetName() > + test.expectedConfigContext.Files[j].GetFileMeta().GetName() + }) + + sort.Slice(result.Files, func(i, j int) bool { + return result.Files[i].GetFileMeta().GetName() > + result.Files[j].GetFileMeta().GetName() + }) + + assert.Truef(t, + protoListEqual(test.expectedConfigContext.Files, result.Files), + "Expect %s Got %s", test.expectedConfigContext.Files, result.Files) assert.Equal(t, test.expectedConfigContext.NAPSysLogServers, result.NAPSysLogServers) assert.Equal(t, test.expectedConfigContext.PlusAPI, result.PlusAPI) assert.ElementsMatch(t, test.expectedConfigContext.AccessLogs, result.AccessLogs) assert.ElementsMatch(t, test.expectedConfigContext.ErrorLogs, result.ErrorLogs) assert.Equal(t, test.expectedConfigContext.StubStatus, result.StubStatus) assert.Equal(t, test.expectedConfigContext.InstanceID, result.InstanceID) + assert.Equal(t, len(test.expectedConfigContext.Files), len(result.Files)) }) } } @@ -1160,3 +1225,14 @@ func TestNginxConfigParser_checkDuplicate(t *testing.T) { }) } } + +func protoListEqual(protoListA, protoListB []*mpi.File) bool { + for i := 0; i < len(protoListA); i++ { + res := proto.Equal(protoListA[i], protoListB[i]) + if !res { + return false + } + } + + return true +} diff --git a/test/config/nginx/nginx-with-multiple-ssl-certs.conf b/test/config/nginx/nginx-with-multiple-ssl-certs.conf new file mode 100644 index 000000000..b74ca2f6a --- /dev/null +++ b/test/config/nginx/nginx-with-multiple-ssl-certs.conf @@ -0,0 +1,49 @@ +worker_processes 1; +error_log %s; +events { + worker_connections 1024; +} + +http { + default_type application/octet-stream; + + log_format main '$remote_addr - $remote_user [$time_local] "$request" ' + '$status $body_bytes_sent "$http_referer" ' + '"$http_user_agent" "$http_x_forwarded_for" ' + '"$bytes_sent" "$request_length" "$request_time" ' + '"$gzip_ratio" $server_protocol '; + + access_log %s main; + + sendfile on; + keepalive_timeout 65; + + server { + listen 8080; + server_name localhost; + + location / { + root /usr/share/nginx/html; + index index.html index.htm; + } + + ssl_certificate %s; + ssl_certificate %s; + + ## + # Enable Metrics + ## + location /api { + stub_status; + allow 127.0.0.1; + deny all; + } + + # redirect server error pages to the static page /50x.html + # + error_page 500 502 503 504 /50x.html; + location = /50x.html { + root /usr/share/nginx/html; + } + } +} diff --git a/test/config/nginx/nginx-with-ssl-certs.conf b/test/config/nginx/nginx-with-ssl-certs.conf new file mode 100644 index 000000000..4d74fbba5 --- /dev/null +++ b/test/config/nginx/nginx-with-ssl-certs.conf @@ -0,0 +1,48 @@ +worker_processes 1; +error_log %s; +events { + worker_connections 1024; +} + +http { + default_type application/octet-stream; + + log_format main '$remote_addr - $remote_user [$time_local] "$request" ' + '$status $body_bytes_sent "$http_referer" ' + '"$http_user_agent" "$http_x_forwarded_for" ' + '"$bytes_sent" "$request_length" "$request_time" ' + '"$gzip_ratio" $server_protocol '; + + access_log %s main; + + sendfile on; + keepalive_timeout 65; + + server { + listen 8080; + server_name localhost; + + location / { + root /usr/share/nginx/html; + index index.html index.htm; + } + + ssl_certificate %s; + + ## + # Enable Metrics + ## + location /api { + stub_status; + allow 127.0.0.1; + deny all; + } + + # redirect server error pages to the static page /50x.html + # + error_page 500 502 503 504 /50x.html; + location = /50x.html { + root /usr/share/nginx/html; + } + } +} diff --git a/test/config/nginx_config.go b/test/config/nginx_config.go index 74e885fcd..e8f6f32ec 100644 --- a/test/config/nginx_config.go +++ b/test/config/nginx_config.go @@ -16,6 +16,12 @@ var embedNginxConfWithMultipleAccessLogs string //go:embed nginx/nginx-not-allowed-dir.conf var embedNginxConfWithNotAllowedDir string +//go:embed nginx/nginx-with-ssl-certs.conf +var embedNginxConfWithSSLCerts string + +//go:embed nginx/nginx-with-multiple-ssl-certs.conf +var embedNginxConfWithMultipleSSLCerts string + //go:embed nginx/nginx-ssl-certs-with-variables.conf var embedNginxConfWithSSLCertsWithVariables string @@ -41,3 +47,11 @@ func GetNginxConfigWithNotAllowedDir(errorLogFile, notAllowedFile, allowedFileDi func GetNginxConfWithSSLCertsWithVariables() string { return embedNginxConfWithSSLCertsWithVariables } + +func GetNginxConfigWithSSLCerts(errorLogFile, accessLogFile, certFile string) string { + return fmt.Sprintf(embedNginxConfWithSSLCerts, errorLogFile, accessLogFile, certFile) +} + +func GetNginxConfigWithMultipleSSLCerts(errorLogFile, accessLogFile, certFile1, certFile2 string) string { + return fmt.Sprintf(embedNginxConfWithMultipleSSLCerts, errorLogFile, accessLogFile, certFile1, certFile2) +} diff --git a/test/model/config.go b/test/model/config.go index 7d5935998..c52dfb0c2 100644 --- a/test/model/config.go +++ b/test/model/config.go @@ -5,7 +5,10 @@ package model -import "github.com/nginx/agent/v3/internal/model" +import ( + mpi "github.com/nginx/agent/v3/api/grpc/mpi/v1" + "github.com/nginx/agent/v3/internal/model" +) func GetConfigContext() *model.NginxConfigContext { return &model.NginxConfigContext{ @@ -116,3 +119,44 @@ func GetConfigContextWithoutErrorLog( NAPSysLogServers: syslogServers, } } + +func GetConfigContextWithFiles( + accessLogName, + errorLogName string, + files []*mpi.File, + instanceID string, + syslogServers []string, +) *model.NginxConfigContext { + return &model.NginxConfigContext{ + StubStatus: &model.APIDetails{ + URL: "", + Listen: "", + Location: "", + }, + PlusAPI: &model.APIDetails{ + URL: "", + Listen: "", + Location: "", + }, + Files: files, + AccessLogs: []*model.AccessLog{ + { + Name: accessLogName, + Format: "$remote_addr - $remote_user [$time_local] \"$request\" $status $body_bytes_sent " + + "\"$http_referer\" \"$http_user_agent\" \"$http_x_forwarded_for\" \"$bytes_sent\" " + + "\"$request_length\" \"$request_time\" \"$gzip_ratio\" $server_protocol ", + Readable: true, + Permissions: "0600", + }, + }, + ErrorLogs: []*model.ErrorLog{ + { + Name: errorLogName, + Readable: true, + Permissions: "0600", + }, + }, + InstanceID: instanceID, + NAPSysLogServers: syslogServers, + } +}