diff --git a/api/grpc/mpi/v1/command.pb.go b/api/grpc/mpi/v1/command.pb.go index b5f0346f4..fbceea13b 100644 --- a/api/grpc/mpi/v1/command.pb.go +++ b/api/grpc/mpi/v1/command.pb.go @@ -8,7 +8,7 @@ // Code generated by protoc-gen-go. DO NOT EDIT. // versions: -// protoc-gen-go v1.36.9 +// protoc-gen-go v1.36.10 // protoc (unknown) // source: mpi/v1/command.proto @@ -2264,7 +2264,9 @@ type APIDetails struct { // the API listen directive Listen string `protobuf:"bytes,2,opt,name=listen,proto3" json:"listen,omitempty"` // the API CA file path - Ca string `protobuf:"bytes,3,opt,name=Ca,proto3" json:"Ca,omitempty"` + Ca string `protobuf:"bytes,3,opt,name=Ca,proto3" json:"Ca,omitempty"` + // flag to know API is configured with 'write=on;' + WriteEnabled bool `protobuf:"varint,4,opt,name=write_enabled,json=writeEnabled,proto3" json:"write_enabled,omitempty"` unknownFields protoimpl.UnknownFields sizeCache protoimpl.SizeCache } @@ -2320,6 +2322,13 @@ func (x *APIDetails) GetCa() string { return "" } +func (x *APIDetails) GetWriteEnabled() bool { + if x != nil { + return x.WriteEnabled + } + return false +} + // A set of runtime NGINX App Protect settings type NGINXAppProtectRuntimeInfo struct { state protoimpl.MessageState `protogen:"open.v1"` @@ -2881,12 +2890,13 @@ const file_mpi_v1_command_proto_rawDesc = "" + "error_logs\x18\x03 \x03(\tR\terrorLogs\x12)\n" + "\x10loadable_modules\x18\x04 \x03(\tR\x0floadableModules\x12'\n" + "\x0fdynamic_modules\x18\x05 \x03(\tR\x0edynamicModules\x12-\n" + - "\bplus_api\x18\x06 \x01(\v2\x12.mpi.v1.APIDetailsR\aplusApi\"P\n" + + "\bplus_api\x18\x06 \x01(\v2\x12.mpi.v1.APIDetailsR\aplusApi\"u\n" + "\n" + "APIDetails\x12\x1a\n" + "\blocation\x18\x01 \x01(\tR\blocation\x12\x16\n" + "\x06listen\x18\x02 \x01(\tR\x06listen\x12\x0e\n" + - "\x02Ca\x18\x03 \x01(\tR\x02Ca\"\xe0\x01\n" + + "\x02Ca\x18\x03 \x01(\tR\x02Ca\x12#\n" + + "\rwrite_enabled\x18\x04 \x01(\bR\fwriteEnabled\"\xe0\x01\n" + "\x1aNGINXAppProtectRuntimeInfo\x12\x18\n" + "\arelease\x18\x01 \x01(\tR\arelease\x128\n" + "\x18attack_signature_version\x18\x02 \x01(\tR\x16attackSignatureVersion\x126\n" + diff --git a/api/grpc/mpi/v1/command.pb.validate.go b/api/grpc/mpi/v1/command.pb.validate.go index 81f716548..d35936df3 100644 --- a/api/grpc/mpi/v1/command.pb.validate.go +++ b/api/grpc/mpi/v1/command.pb.validate.go @@ -4895,6 +4895,8 @@ func (m *APIDetails) validate(all bool) error { // no validation rules for Ca + // no validation rules for WriteEnabled + if len(errors) > 0 { return APIDetailsMultiError(errors) } diff --git a/api/grpc/mpi/v1/command.proto b/api/grpc/mpi/v1/command.proto index 84fb6a020..894e64acd 100644 --- a/api/grpc/mpi/v1/command.proto +++ b/api/grpc/mpi/v1/command.proto @@ -354,6 +354,8 @@ message APIDetails { string listen = 2; // the API CA file path string Ca = 3; + // flag to know API is configured with 'write=on;' + bool write_enabled = 4; } // A set of runtime NGINX App Protect settings diff --git a/api/grpc/mpi/v1/common.pb.go b/api/grpc/mpi/v1/common.pb.go index e6d06cf37..b59f47b5a 100644 --- a/api/grpc/mpi/v1/common.pb.go +++ b/api/grpc/mpi/v1/common.pb.go @@ -5,7 +5,7 @@ // Code generated by protoc-gen-go. DO NOT EDIT. // versions: -// protoc-gen-go v1.36.9 +// protoc-gen-go v1.36.10 // protoc (unknown) // source: mpi/v1/common.proto diff --git a/api/grpc/mpi/v1/files.pb.go b/api/grpc/mpi/v1/files.pb.go index 47d1362b7..0223bae3a 100644 --- a/api/grpc/mpi/v1/files.pb.go +++ b/api/grpc/mpi/v1/files.pb.go @@ -5,7 +5,7 @@ // Code generated by protoc-gen-go. DO NOT EDIT. // versions: -// protoc-gen-go v1.36.9 +// protoc-gen-go v1.36.10 // protoc (unknown) // source: mpi/v1/files.proto diff --git a/docs/proto/protos.md b/docs/proto/protos.md index 3ae57f5e4..529629ff3 100644 --- a/docs/proto/protos.md +++ b/docs/proto/protos.md @@ -697,6 +697,7 @@ Perform an associated API action on an instance | location | [string](#string) | | the API location directive | | listen | [string](#string) | | the API listen directive | | Ca | [string](#string) | | the API CA file path | +| write_enabled | [bool](#bool) | | flag to know API is configured with 'write=on;' | diff --git a/internal/datasource/config/nginx_config_parser.go b/internal/datasource/config/nginx_config_parser.go index 5b0593f28..7653d4174 100644 --- a/internal/datasource/config/nginx_config_parser.go +++ b/internal/datasource/config/nginx_config_parser.go @@ -67,6 +67,15 @@ type ( current *crossplane.Directive, apiType string) []*model.APIDetails ) +type createAPIDetailsParams struct { + locationDirectiveName string + address string + path string + caCertLocation string + isSSL bool + writeEnabled bool +} + func NewNginxConfigParser(agentConfig *config.Config) *NginxConfigParser { return &NginxConfigParser{ agentConfig: agentConfig, @@ -251,6 +260,8 @@ func (ncp *NginxConfigParser) createNginxConfigContext( nginxConfigContext.PlusAPIs = append(nginxConfigContext.PlusAPIs, plusAPIs...) } + nginxConfigContext.PlusAPIs = ncp.sortPlusAPIs(nginxConfigContext.PlusAPIs) + if len(napSyslogServersFound) > 0 { var napSyslogServer []string for server := range napSyslogServersFound { @@ -699,11 +710,22 @@ func (ncp *NginxConfigParser) apiDetailsFromLocationDirective( addresses := ncp.parseAddressFromServerDirective(parent) path := ncp.parsePathFromLocationDirective(current) + writeEnabled := ncp.isWriteEnabled(locChild) + if locChild.Directive == locationDirectiveName { for _, address := range addresses { + params := createAPIDetailsParams{ + locationDirectiveName: locationDirectiveName, + address: address, + path: path, + caCertLocation: caCertLocation, + isSSL: isSSL, + writeEnabled: writeEnabled, + } + details = append( details, - ncp.createAPIDetails(locationDirectiveName, address, path, caCertLocation, isSSL), + ncp.createAPIDetails(params), ) } } @@ -713,28 +735,30 @@ func (ncp *NginxConfigParser) apiDetailsFromLocationDirective( } func (ncp *NginxConfigParser) createAPIDetails( - locationDirectiveName, address, path, caCertLocation string, isSSL bool, + params createAPIDetailsParams, ) (details *model.APIDetails) { - if strings.HasPrefix(address, "unix:") { + if strings.HasPrefix(params.address, "unix:") { format := unixStubStatusFormat - if locationDirectiveName == plusAPIDirective { + if params.locationDirectiveName == plusAPIDirective { format = unixPlusAPIFormat } details = &model.APIDetails{ - URL: fmt.Sprintf(format, path), - Listen: address, - Location: path, - Ca: caCertLocation, + URL: fmt.Sprintf(format, params.path), + Listen: params.address, + Location: params.path, + Ca: params.caCertLocation, + WriteEnabled: params.writeEnabled, } } else { details = &model.APIDetails{ - URL: fmt.Sprintf("%s://%s%s", map[bool]string{true: "https", false: "http"}[isSSL], - address, path), - Listen: address, - Location: path, - Ca: caCertLocation, + URL: fmt.Sprintf("%s://%s%s", map[bool]string{true: "https", false: "http"}[params.isSSL], + params.address, params.path), + Listen: params.address, + Location: params.path, + Ca: params.caCertLocation, + WriteEnabled: params.writeEnabled, } } @@ -888,3 +912,33 @@ func (ncp *NginxConfigParser) isDuplicateFile(nginxConfigContextFiles []*mpi.Fil return false } + +func (ncp *NginxConfigParser) isWriteEnabled(locChild *crossplane.Directive) bool { + if locChild.Directive != plusAPIDirective { + return false + } + + for _, arg := range locChild.Args { + if strings.EqualFold(arg, "write=on") { + return true + } + } + + return false +} + +func (ncp *NginxConfigParser) sortPlusAPIs(apis []*model.APIDetails) []*model.APIDetails { + slices.SortFunc(apis, func(a, b *model.APIDetails) int { + if a.WriteEnabled && !b.WriteEnabled { + return -1 + } + + if b.WriteEnabled && !a.WriteEnabled { + return 1 + } + + return 0 + }) + + return apis +} diff --git a/internal/datasource/config/nginx_config_parser_test.go b/internal/datasource/config/nginx_config_parser_test.go index 8051a5200..a9aa617ac 100644 --- a/internal/datasource/config/nginx_config_parser_test.go +++ b/internal/datasource/config/nginx_config_parser_test.go @@ -150,7 +150,7 @@ var ( listen [::]:8000; server_name _; location /api/ { - api write=on; + api write=off; allow 127.0.0.1; deny all; } @@ -275,7 +275,7 @@ var ( } location /api { - api write=on; + api write=off; } } @@ -286,6 +286,14 @@ server { return 418; } ` + testConf23 = `server { + listen 127.0.0.1:9090; + location /writeapi { + api write=on; + allow 127.0.0.1; + deny all; + } +}` ) //nolint:maintidx // The test cannot be refactored @@ -815,108 +823,120 @@ func TestNginxConfigParser_urlsForLocationDirective(t *testing.T) { }{ { plus: &model.APIDetails{ - URL: "http://localhost:80/api/", - Listen: "localhost:80", - Location: "/api/", + URL: "http://localhost:80/api/", + Listen: "localhost:80", + Location: "/api/", + WriteEnabled: true, }, name: "Test 1: listen localhost 80, allow 127.0.0.1 - Plus", conf: testConf01, }, { plus: &model.APIDetails{ - URL: "http://localhost:80/api/", - Listen: "localhost:80", - Location: "/api/", + URL: "http://localhost:80/api/", + Listen: "localhost:80", + Location: "/api/", + WriteEnabled: true, }, name: "Test 2: listen *:80 - Plus", conf: testConf02, }, { plus: &model.APIDetails{ - URL: "http://localhost:80/api/", - Listen: "localhost:80", - Location: "/api/", + URL: "http://localhost:80/api/", + Listen: "localhost:80", + Location: "/api/", + WriteEnabled: true, }, name: "Test 3: server_name _ - Plus", conf: testConf03, }, { plus: &model.APIDetails{ - URL: "http://localhost:8888/api/", - Listen: "localhost:8888", - Location: "/api/", + URL: "http://localhost:8888/api/", + Listen: "localhost:8888", + Location: "/api/", + WriteEnabled: true, }, name: "Test 4: server_name status.internal.com - Plus", conf: testConf04, }, { plus: &model.APIDetails{ - URL: "http://localhost:8080/privateapi", - Listen: "localhost:8080", - Location: "/privateapi", + URL: "http://localhost:8080/privateapi", + Listen: "localhost:8080", + Location: "/privateapi", + WriteEnabled: true, }, name: "Test 5: location /privateapi - Plus", conf: testConf05, }, { plus: &model.APIDetails{ - URL: "http://localhost:80/api/", - Listen: "localhost:80", - Location: "/api/", + URL: "http://localhost:80/api/", + Listen: "localhost:80", + Location: "/api/", + WriteEnabled: true, }, name: "Test 6: listen [::]:80 default_server - Plus", conf: testConf06, }, { plus: &model.APIDetails{ - URL: "http://localhost:80/api/", - Listen: "localhost:80", - Location: "/api/", + URL: "http://localhost:80/api/", + Listen: "localhost:80", + Location: "/api/", + WriteEnabled: true, }, name: "Test 7: listen 127.0.0.1, server_name _ - Plus", conf: testConf07, }, { plus: &model.APIDetails{ - URL: "http://localhost:80/api/", - Listen: "localhost:80", - Location: "/api/", + URL: "http://localhost:80/api/", + Listen: "localhost:80", + Location: "/api/", + WriteEnabled: true, }, name: "Test 8: location = /api/, listen 127.0.0.1 - Plus", conf: testConf08, }, { plus: &model.APIDetails{ - URL: "http://localhost:80/api/", - Listen: "localhost:80", - Location: "/api/", + URL: "http://localhost:80/api/", + Listen: "localhost:80", + Location: "/api/", + WriteEnabled: true, }, name: "Test 9: location = /api/ , listen 80 - Plus", conf: testConf09, }, { plus: &model.APIDetails{ - URL: "http://localhost:80/api/", - Listen: "localhost:80", - Location: "/api/", + URL: "http://localhost:80/api/", + Listen: "localhost:80", + Location: "/api/", + WriteEnabled: true, }, name: "Test 10: listen :80 - Plus", conf: testConf10, }, { plus: &model.APIDetails{ - URL: "http://localhost:80/api/", - Listen: "localhost:80", - Location: "/api/", + URL: "http://localhost:80/api/", + Listen: "localhost:80", + Location: "/api/", + WriteEnabled: true, }, name: "Test 11: listen localhost - Plus", conf: testConf11, }, { plus: &model.APIDetails{ - URL: "http://localhost:80/api/", - Listen: "localhost:80", - Location: "/api/", + URL: "http://localhost:80/api/", + Listen: "localhost:80", + Location: "/api/", + WriteEnabled: true, }, name: "Test 12: listen [::1] - Plus", conf: testConf12, @@ -973,9 +993,10 @@ func TestNginxConfigParser_urlsForLocationDirective(t *testing.T) { Location: "/stub_status", }, plus: &model.APIDetails{ - URL: "http://localhost:80/api/", - Listen: "localhost:80", - Location: "/api/", + URL: "http://localhost:80/api/", + Listen: "localhost:80", + Location: "/api/", + WriteEnabled: true, }, name: "Test 18: listen 80 - OSS & Plus", conf: testConf18, @@ -1000,9 +1021,10 @@ func TestNginxConfigParser_urlsForLocationDirective(t *testing.T) { }, { plus: &model.APIDetails{ - URL: "http://nginx-plus-api/api", - Listen: "unix:/var/run/nginx/nginx-plus-api.sock", - Location: "/api", + URL: "http://nginx-plus-api/api", + Listen: "unix:/var/run/nginx/nginx-plus-api.sock", + Location: "/api", + WriteEnabled: true, }, name: "Test 21: listen unix:/var/run/nginx/nginx-plus-api.sock - Plus Unix Socket", conf: testConf21, @@ -1016,6 +1038,16 @@ func TestNginxConfigParser_urlsForLocationDirective(t *testing.T) { name: "Test 22: Multiple Plus Unix Sockets", conf: testConf22, }, + { + plus: &model.APIDetails{ + URL: "http://localhost:9090/writeapi", + Listen: "localhost:9090", + Location: "/writeapi", + WriteEnabled: true, + }, + name: "Test 23: Explicitly Write-Enabled Plus API", + conf: testConf23, + }, } for _, test := range tests { diff --git a/internal/datasource/proto/instance.go b/internal/datasource/proto/instance.go index 38c802030..61f54a68b 100644 --- a/internal/datasource/proto/instance.go +++ b/internal/datasource/proto/instance.go @@ -20,7 +20,8 @@ func NginxPlusRuntimeInfoEqual(nginxPlusRuntimeInfo *mpi.NGINXPlusRuntimeInfo, nginxPlusRuntimeInfo.GetStubStatus().GetListen() != nginxConfigContext.StubStatus.Listen || nginxPlusRuntimeInfo.GetPlusApi().GetListen() != nginxConfigContext.PlusAPI.Listen || nginxPlusRuntimeInfo.GetStubStatus().GetLocation() != nginxConfigContext.StubStatus.Location || - nginxPlusRuntimeInfo.GetPlusApi().GetLocation() != nginxConfigContext.PlusAPI.Location { + nginxPlusRuntimeInfo.GetPlusApi().GetLocation() != nginxConfigContext.PlusAPI.Location || + nginxPlusRuntimeInfo.GetPlusApi().GetWriteEnabled() != nginxConfigContext.PlusAPI.WriteEnabled { return false } @@ -59,6 +60,7 @@ func UpdateNginxInstanceRuntime( nginxPlusRuntimeInfo.PlusApi.Listen = nginxConfigContext.PlusAPI.Listen nginxPlusRuntimeInfo.StubStatus.Location = nginxConfigContext.StubStatus.Location nginxPlusRuntimeInfo.PlusApi.Location = nginxConfigContext.PlusAPI.Location + nginxPlusRuntimeInfo.PlusApi.WriteEnabled = nginxConfigContext.PlusAPI.WriteEnabled updatesRequired = true } } else { diff --git a/internal/datasource/proto/instance_test.go b/internal/datasource/proto/instance_test.go index 0ad5c25a8..90e1cf303 100644 --- a/internal/datasource/proto/instance_test.go +++ b/internal/datasource/proto/instance_test.go @@ -44,9 +44,32 @@ func TestInstanceWatcherService_updateNginxInstanceRuntime(t *testing.T) { }, }, PlusAPI: &model.APIDetails{ + URL: "http://127.0.0.1:8081/api", + Listen: "", + WriteEnabled: false, + }, + StubStatus: &model.APIDetails{ URL: "http://127.0.0.1:8081/api", Listen: "", }, + } + + nginxPlusConfigContextWrite := &model.NginxConfigContext{ + AccessLogs: []*model.AccessLog{ + { + Name: "/usr/local/var/log/nginx/access.log", + }, + }, + ErrorLogs: []*model.ErrorLog{ + { + Name: "/usr/local/var/log/nginx/error.log", + }, + }, + PlusAPI: &model.APIDetails{ + URL: "http://127.0.0.1:8081/api/write", + Listen: "8080", + WriteEnabled: true, + }, StubStatus: &model.APIDetails{ URL: "http://127.0.0.1:8081/api", Listen: "", @@ -68,12 +91,26 @@ func TestInstanceWatcherService_updateNginxInstanceRuntime(t *testing.T) { nginxConfigContext: nginxPlusConfigContext, instance: protos.NginxPlusInstance([]string{}), }, + { + name: "Test 3: Plus Instance (Write-Enabled)", + nginxConfigContext: nginxPlusConfigContextWrite, + instance: protos.NginxPlusInstance([]string{}), + }, } for _, test := range tests { t.Run(test.name, func(tt *testing.T) { UpdateNginxInstanceRuntime(test.instance, test.nginxConfigContext) - if test.name == "Test 2: Plus Instance" { + if test.name == "Test 1: OSS Instance" { + assert.Equal(t, test.nginxConfigContext.AccessLogs[0].Name, test.instance.GetInstanceRuntime(). + GetNginxRuntimeInfo().GetAccessLogs()[0]) + assert.Equal(t, test.nginxConfigContext.ErrorLogs[0].Name, test.instance.GetInstanceRuntime(). + GetNginxRuntimeInfo().GetErrorLogs()[0]) + assert.Equal(t, test.nginxConfigContext.StubStatus.Location, test.instance.GetInstanceRuntime(). + GetNginxRuntimeInfo().GetStubStatus().GetLocation()) + assert.Equal(t, test.nginxConfigContext.StubStatus.Listen, test.instance.GetInstanceRuntime(). + GetNginxRuntimeInfo().GetStubStatus().GetListen()) + } else { assert.Equal(t, test.nginxConfigContext.AccessLogs[0].Name, test.instance.GetInstanceRuntime(). GetNginxPlusRuntimeInfo().GetAccessLogs()[0]) assert.Equal(t, test.nginxConfigContext.ErrorLogs[0].Name, test.instance.GetInstanceRuntime(). @@ -86,15 +123,8 @@ func TestInstanceWatcherService_updateNginxInstanceRuntime(t *testing.T) { GetNginxPlusRuntimeInfo().GetStubStatus().GetListen()) assert.Equal(t, test.nginxConfigContext.PlusAPI.Listen, test.instance.GetInstanceRuntime(). GetNginxPlusRuntimeInfo().GetPlusApi().GetListen()) - } else { - assert.Equal(t, test.nginxConfigContext.AccessLogs[0].Name, test.instance.GetInstanceRuntime(). - GetNginxRuntimeInfo().GetAccessLogs()[0]) - assert.Equal(t, test.nginxConfigContext.ErrorLogs[0].Name, test.instance.GetInstanceRuntime(). - GetNginxRuntimeInfo().GetErrorLogs()[0]) - assert.Equal(t, test.nginxConfigContext.StubStatus.Location, test.instance.GetInstanceRuntime(). - GetNginxRuntimeInfo().GetStubStatus().GetLocation()) - assert.Equal(t, test.nginxConfigContext.StubStatus.Listen, test.instance.GetInstanceRuntime(). - GetNginxRuntimeInfo().GetStubStatus().GetListen()) + assert.Equal(t, test.nginxConfigContext.PlusAPI.WriteEnabled, test.instance.GetInstanceRuntime(). + GetNginxPlusRuntimeInfo().GetPlusApi().GetWriteEnabled()) } }) } diff --git a/internal/model/config.go b/internal/model/config.go index 1aaf95c97..5455f61d1 100644 --- a/internal/model/config.go +++ b/internal/model/config.go @@ -26,10 +26,11 @@ type NginxConfigContext struct { } type APIDetails struct { - URL string - Listen string - Location string - Ca string + URL string + Listen string + Location string + Ca string + WriteEnabled bool } type ManifestFile struct { @@ -95,7 +96,8 @@ func (ncc *NginxConfigContext) Equal(otherNginxConfigContext *NginxConfigContext if ncc.PlusAPI != nil && otherNginxConfigContext.PlusAPI != nil { if ncc.PlusAPI.URL != otherNginxConfigContext.PlusAPI.URL || ncc.PlusAPI.Listen != otherNginxConfigContext.PlusAPI.Listen || ncc.PlusAPI.Location != - otherNginxConfigContext.PlusAPI.Location { + otherNginxConfigContext.PlusAPI.Location || + ncc.PlusAPI.WriteEnabled != otherNginxConfigContext.PlusAPI.WriteEnabled { return false } }