From 5b314940f6ee24d86a05a1fe2dccf83ff233110f Mon Sep 17 00:00:00 2001 From: Aphral Griffin Date: Wed, 11 Jun 2025 09:38:18 +0100 Subject: [PATCH 01/17] add auxiliary command server proto --- api/grpc/mpi/v1/command.pb.go | 185 +++++++++++++++------ api/grpc/mpi/v1/command.pb.validate.go | 218 +++++++++++++++++++++++++ api/grpc/mpi/v1/command.proto | 16 +- docs/proto/protos.md | 19 +++ 4 files changed, 385 insertions(+), 53 deletions(-) diff --git a/api/grpc/mpi/v1/command.pb.go b/api/grpc/mpi/v1/command.pb.go index 321c347a9..485adec14 100644 --- a/api/grpc/mpi/v1/command.pb.go +++ b/api/grpc/mpi/v1/command.pb.go @@ -2427,8 +2427,10 @@ type AgentConfig struct { Features []string `protobuf:"bytes,5,rep,name=features,proto3" json:"features,omitempty"` // Message buffer size, maximum not acknowledged messages from the subscribe perspective MessageBufferSize string `protobuf:"bytes,6,opt,name=message_buffer_size,json=messageBufferSize,proto3" json:"message_buffer_size,omitempty"` - unknownFields protoimpl.UnknownFields - sizeCache protoimpl.SizeCache + // Auxiliary Command server settings + AuxiliaryCommand *AuxiliaryCommandServer `protobuf:"bytes,7,opt,name=auxiliary_command,json=auxiliaryCommand,proto3" json:"auxiliary_command,omitempty"` + unknownFields protoimpl.UnknownFields + sizeCache protoimpl.SizeCache } func (x *AgentConfig) Reset() { @@ -2503,6 +2505,13 @@ func (x *AgentConfig) GetMessageBufferSize() string { return "" } +func (x *AgentConfig) GetAuxiliaryCommand() *AuxiliaryCommandServer { + if x != nil { + return x.AuxiliaryCommand + } + return nil +} + // The command server settings, associated with messaging from an external source type CommandServer struct { state protoimpl.MessageState `protogen:"open.v1"` @@ -2567,6 +2576,70 @@ func (x *CommandServer) GetTls() *TLSSettings { return nil } +// The auxiliary server settings, associated with messaging from an external source +type AuxiliaryCommandServer struct { + state protoimpl.MessageState `protogen:"open.v1"` + // Server configuration (e.g., host, port, type) + Server *ServerSettings `protobuf:"bytes,1,opt,name=server,proto3" json:"server,omitempty"` + // Authentication configuration (e.g., token) + Auth *AuthSettings `protobuf:"bytes,2,opt,name=auth,proto3" json:"auth,omitempty"` + // TLS configuration for secure communication + Tls *TLSSettings `protobuf:"bytes,3,opt,name=tls,proto3" json:"tls,omitempty"` + unknownFields protoimpl.UnknownFields + sizeCache protoimpl.SizeCache +} + +func (x *AuxiliaryCommandServer) Reset() { + *x = AuxiliaryCommandServer{} + mi := &file_mpi_v1_command_proto_msgTypes[37] + ms := protoimpl.X.MessageStateOf(protoimpl.Pointer(x)) + ms.StoreMessageInfo(mi) +} + +func (x *AuxiliaryCommandServer) String() string { + return protoimpl.X.MessageStringOf(x) +} + +func (*AuxiliaryCommandServer) ProtoMessage() {} + +func (x *AuxiliaryCommandServer) ProtoReflect() protoreflect.Message { + mi := &file_mpi_v1_command_proto_msgTypes[37] + if x != nil { + ms := protoimpl.X.MessageStateOf(protoimpl.Pointer(x)) + if ms.LoadMessageInfo() == nil { + ms.StoreMessageInfo(mi) + } + return ms + } + return mi.MessageOf(x) +} + +// Deprecated: Use AuxiliaryCommandServer.ProtoReflect.Descriptor instead. +func (*AuxiliaryCommandServer) Descriptor() ([]byte, []int) { + return file_mpi_v1_command_proto_rawDescGZIP(), []int{37} +} + +func (x *AuxiliaryCommandServer) GetServer() *ServerSettings { + if x != nil { + return x.Server + } + return nil +} + +func (x *AuxiliaryCommandServer) GetAuth() *AuthSettings { + if x != nil { + return x.Auth + } + return nil +} + +func (x *AuxiliaryCommandServer) GetTls() *TLSSettings { + if x != nil { + return x.Tls + } + return nil +} + // The metrics settings associated with origins (sources) of the metrics and destinations (exporter) type MetricsServer struct { state protoimpl.MessageState `protogen:"open.v1"` @@ -2576,7 +2649,7 @@ type MetricsServer struct { func (x *MetricsServer) Reset() { *x = MetricsServer{} - mi := &file_mpi_v1_command_proto_msgTypes[37] + mi := &file_mpi_v1_command_proto_msgTypes[38] ms := protoimpl.X.MessageStateOf(protoimpl.Pointer(x)) ms.StoreMessageInfo(mi) } @@ -2588,7 +2661,7 @@ func (x *MetricsServer) String() string { func (*MetricsServer) ProtoMessage() {} func (x *MetricsServer) ProtoReflect() protoreflect.Message { - mi := &file_mpi_v1_command_proto_msgTypes[37] + mi := &file_mpi_v1_command_proto_msgTypes[38] if x != nil { ms := protoimpl.X.MessageStateOf(protoimpl.Pointer(x)) if ms.LoadMessageInfo() == nil { @@ -2601,7 +2674,7 @@ func (x *MetricsServer) ProtoReflect() protoreflect.Message { // Deprecated: Use MetricsServer.ProtoReflect.Descriptor instead. func (*MetricsServer) Descriptor() ([]byte, []int) { - return file_mpi_v1_command_proto_rawDescGZIP(), []int{37} + return file_mpi_v1_command_proto_rawDescGZIP(), []int{38} } // The file settings associated with file server for configurations @@ -2613,7 +2686,7 @@ type FileServer struct { func (x *FileServer) Reset() { *x = FileServer{} - mi := &file_mpi_v1_command_proto_msgTypes[38] + mi := &file_mpi_v1_command_proto_msgTypes[39] ms := protoimpl.X.MessageStateOf(protoimpl.Pointer(x)) ms.StoreMessageInfo(mi) } @@ -2625,7 +2698,7 @@ func (x *FileServer) String() string { func (*FileServer) ProtoMessage() {} func (x *FileServer) ProtoReflect() protoreflect.Message { - mi := &file_mpi_v1_command_proto_msgTypes[38] + mi := &file_mpi_v1_command_proto_msgTypes[39] if x != nil { ms := protoimpl.X.MessageStateOf(protoimpl.Pointer(x)) if ms.LoadMessageInfo() == nil { @@ -2638,7 +2711,7 @@ func (x *FileServer) ProtoReflect() protoreflect.Message { // Deprecated: Use FileServer.ProtoReflect.Descriptor instead. func (*FileServer) Descriptor() ([]byte, []int) { - return file_mpi_v1_command_proto_rawDescGZIP(), []int{38} + return file_mpi_v1_command_proto_rawDescGZIP(), []int{39} } var File_mpi_v1_command_proto protoreflect.FileDescriptor @@ -2798,17 +2871,22 @@ const file_mpi_v1_command_proto_rawDesc = "" + "\arelease\x18\x01 \x01(\tR\arelease\x128\n" + "\x18attack_signature_version\x18\x02 \x01(\tR\x16attackSignatureVersion\x126\n" + "\x17threat_campaign_version\x18\x03 \x01(\tR\x15threatCampaignVersion\"\x10\n" + - "\x0eInstanceAction\"\x94\x02\n" + + "\x0eInstanceAction\"\xe1\x02\n" + "\vAgentConfig\x12/\n" + "\acommand\x18\x01 \x01(\v2\x15.mpi.v1.CommandServerR\acommand\x12/\n" + "\ametrics\x18\x02 \x01(\v2\x15.mpi.v1.MetricsServerR\ametrics\x12&\n" + "\x04file\x18\x03 \x01(\v2\x12.mpi.v1.FileServerR\x04file\x12/\n" + "\x06labels\x18\x04 \x03(\v2\x17.google.protobuf.StructR\x06labels\x12\x1a\n" + "\bfeatures\x18\x05 \x03(\tR\bfeatures\x12.\n" + - "\x13message_buffer_size\x18\x06 \x01(\tR\x11messageBufferSize\"\x90\x01\n" + + "\x13message_buffer_size\x18\x06 \x01(\tR\x11messageBufferSize\x12K\n" + + "\x11auxiliary_command\x18\a \x01(\v2\x1e.mpi.v1.AuxiliaryCommandServerR\x10auxiliaryCommand\"\x90\x01\n" + "\rCommandServer\x12.\n" + "\x06server\x18\x01 \x01(\v2\x16.mpi.v1.ServerSettingsR\x06server\x12(\n" + "\x04auth\x18\x02 \x01(\v2\x14.mpi.v1.AuthSettingsR\x04auth\x12%\n" + + "\x03tls\x18\x03 \x01(\v2\x13.mpi.v1.TLSSettingsR\x03tls\"\x99\x01\n" + + "\x16AuxiliaryCommandServer\x12.\n" + + "\x06server\x18\x01 \x01(\v2\x16.mpi.v1.ServerSettingsR\x06server\x12(\n" + + "\x04auth\x18\x02 \x01(\v2\x14.mpi.v1.AuthSettingsR\x04auth\x12%\n" + "\x03tls\x18\x03 \x01(\v2\x13.mpi.v1.TLSSettingsR\x03tls\"\x0f\n" + "\rMetricsServer\"\f\n" + "\n" + @@ -2832,7 +2910,7 @@ func file_mpi_v1_command_proto_rawDescGZIP() []byte { } var file_mpi_v1_command_proto_enumTypes = make([]protoimpl.EnumInfo, 2) -var file_mpi_v1_command_proto_msgTypes = make([]protoimpl.MessageInfo, 39) +var file_mpi_v1_command_proto_msgTypes = make([]protoimpl.MessageInfo, 40) var file_mpi_v1_command_proto_goTypes = []any{ (InstanceHealth_InstanceHealthStatus)(0), // 0: mpi.v1.InstanceHealth.InstanceHealthStatus (InstanceMeta_InstanceType)(0), // 1: mpi.v1.InstanceMeta.InstanceType @@ -2873,50 +2951,51 @@ var file_mpi_v1_command_proto_goTypes = []any{ (*InstanceAction)(nil), // 36: mpi.v1.InstanceAction (*AgentConfig)(nil), // 37: mpi.v1.AgentConfig (*CommandServer)(nil), // 38: mpi.v1.CommandServer - (*MetricsServer)(nil), // 39: mpi.v1.MetricsServer - (*FileServer)(nil), // 40: mpi.v1.FileServer - (*MessageMeta)(nil), // 41: mpi.v1.MessageMeta - (*CommandResponse)(nil), // 42: mpi.v1.CommandResponse - (*FileOverview)(nil), // 43: mpi.v1.FileOverview - (*structpb.Struct)(nil), // 44: google.protobuf.Struct - (*ServerSettings)(nil), // 45: mpi.v1.ServerSettings - (*AuthSettings)(nil), // 46: mpi.v1.AuthSettings - (*TLSSettings)(nil), // 47: mpi.v1.TLSSettings + (*AuxiliaryCommandServer)(nil), // 39: mpi.v1.AuxiliaryCommandServer + (*MetricsServer)(nil), // 40: mpi.v1.MetricsServer + (*FileServer)(nil), // 41: mpi.v1.FileServer + (*MessageMeta)(nil), // 42: mpi.v1.MessageMeta + (*CommandResponse)(nil), // 43: mpi.v1.CommandResponse + (*FileOverview)(nil), // 44: mpi.v1.FileOverview + (*structpb.Struct)(nil), // 45: google.protobuf.Struct + (*ServerSettings)(nil), // 46: mpi.v1.ServerSettings + (*AuthSettings)(nil), // 47: mpi.v1.AuthSettings + (*TLSSettings)(nil), // 48: mpi.v1.TLSSettings } var file_mpi_v1_command_proto_depIdxs = []int32{ - 41, // 0: mpi.v1.CreateConnectionRequest.message_meta:type_name -> mpi.v1.MessageMeta + 42, // 0: mpi.v1.CreateConnectionRequest.message_meta:type_name -> mpi.v1.MessageMeta 3, // 1: mpi.v1.CreateConnectionRequest.resource:type_name -> mpi.v1.Resource 27, // 2: mpi.v1.Resource.instances:type_name -> mpi.v1.Instance 4, // 3: mpi.v1.Resource.host_info:type_name -> mpi.v1.HostInfo 6, // 4: mpi.v1.Resource.container_info:type_name -> mpi.v1.ContainerInfo 5, // 5: mpi.v1.HostInfo.release_info:type_name -> mpi.v1.ReleaseInfo 5, // 6: mpi.v1.ContainerInfo.release_info:type_name -> mpi.v1.ReleaseInfo - 42, // 7: mpi.v1.CreateConnectionResponse.response:type_name -> mpi.v1.CommandResponse + 43, // 7: mpi.v1.CreateConnectionResponse.response:type_name -> mpi.v1.CommandResponse 37, // 8: mpi.v1.CreateConnectionResponse.agent_config:type_name -> mpi.v1.AgentConfig - 41, // 9: mpi.v1.UpdateDataPlaneStatusRequest.message_meta:type_name -> mpi.v1.MessageMeta + 42, // 9: mpi.v1.UpdateDataPlaneStatusRequest.message_meta:type_name -> mpi.v1.MessageMeta 3, // 10: mpi.v1.UpdateDataPlaneStatusRequest.resource:type_name -> mpi.v1.Resource 0, // 11: mpi.v1.InstanceHealth.instance_health_status:type_name -> mpi.v1.InstanceHealth.InstanceHealthStatus - 41, // 12: mpi.v1.UpdateDataPlaneHealthRequest.message_meta:type_name -> mpi.v1.MessageMeta + 42, // 12: mpi.v1.UpdateDataPlaneHealthRequest.message_meta:type_name -> mpi.v1.MessageMeta 10, // 13: mpi.v1.UpdateDataPlaneHealthRequest.instance_healths:type_name -> mpi.v1.InstanceHealth - 41, // 14: mpi.v1.DataPlaneResponse.message_meta:type_name -> mpi.v1.MessageMeta - 42, // 15: mpi.v1.DataPlaneResponse.command_response:type_name -> mpi.v1.CommandResponse - 41, // 16: mpi.v1.ManagementPlaneRequest.message_meta:type_name -> mpi.v1.MessageMeta + 42, // 14: mpi.v1.DataPlaneResponse.message_meta:type_name -> mpi.v1.MessageMeta + 43, // 15: mpi.v1.DataPlaneResponse.command_response:type_name -> mpi.v1.CommandResponse + 42, // 16: mpi.v1.ManagementPlaneRequest.message_meta:type_name -> mpi.v1.MessageMeta 15, // 17: mpi.v1.ManagementPlaneRequest.status_request:type_name -> mpi.v1.StatusRequest 16, // 18: mpi.v1.ManagementPlaneRequest.health_request:type_name -> mpi.v1.HealthRequest 17, // 19: mpi.v1.ManagementPlaneRequest.config_apply_request:type_name -> mpi.v1.ConfigApplyRequest 18, // 20: mpi.v1.ManagementPlaneRequest.config_upload_request:type_name -> mpi.v1.ConfigUploadRequest 19, // 21: mpi.v1.ManagementPlaneRequest.action_request:type_name -> mpi.v1.APIActionRequest 26, // 22: mpi.v1.ManagementPlaneRequest.command_status_request:type_name -> mpi.v1.CommandStatusRequest - 43, // 23: mpi.v1.ConfigApplyRequest.overview:type_name -> mpi.v1.FileOverview - 43, // 24: mpi.v1.ConfigUploadRequest.overview:type_name -> mpi.v1.FileOverview + 44, // 23: mpi.v1.ConfigApplyRequest.overview:type_name -> mpi.v1.FileOverview + 44, // 24: mpi.v1.ConfigUploadRequest.overview:type_name -> mpi.v1.FileOverview 20, // 25: mpi.v1.APIActionRequest.nginx_plus_action:type_name -> mpi.v1.NGINXPlusAction 21, // 26: mpi.v1.NGINXPlusAction.update_http_upstream_servers:type_name -> mpi.v1.UpdateHTTPUpstreamServers 22, // 27: mpi.v1.NGINXPlusAction.get_http_upstream_servers:type_name -> mpi.v1.GetHTTPUpstreamServers 23, // 28: mpi.v1.NGINXPlusAction.update_stream_servers:type_name -> mpi.v1.UpdateStreamServers 24, // 29: mpi.v1.NGINXPlusAction.get_upstreams:type_name -> mpi.v1.GetUpstreams 25, // 30: mpi.v1.NGINXPlusAction.get_stream_upstreams:type_name -> mpi.v1.GetStreamUpstreams - 44, // 31: mpi.v1.UpdateHTTPUpstreamServers.servers:type_name -> google.protobuf.Struct - 44, // 32: mpi.v1.UpdateStreamServers.servers:type_name -> google.protobuf.Struct + 45, // 31: mpi.v1.UpdateHTTPUpstreamServers.servers:type_name -> google.protobuf.Struct + 45, // 32: mpi.v1.UpdateStreamServers.servers:type_name -> google.protobuf.Struct 28, // 33: mpi.v1.Instance.instance_meta:type_name -> mpi.v1.InstanceMeta 29, // 34: mpi.v1.Instance.instance_config:type_name -> mpi.v1.InstanceConfig 30, // 35: mpi.v1.Instance.instance_runtime:type_name -> mpi.v1.InstanceRuntime @@ -2931,25 +3010,29 @@ var file_mpi_v1_command_proto_depIdxs = []int32{ 34, // 44: mpi.v1.NGINXPlusRuntimeInfo.stub_status:type_name -> mpi.v1.APIDetails 34, // 45: mpi.v1.NGINXPlusRuntimeInfo.plus_api:type_name -> mpi.v1.APIDetails 38, // 46: mpi.v1.AgentConfig.command:type_name -> mpi.v1.CommandServer - 39, // 47: mpi.v1.AgentConfig.metrics:type_name -> mpi.v1.MetricsServer - 40, // 48: mpi.v1.AgentConfig.file:type_name -> mpi.v1.FileServer - 44, // 49: mpi.v1.AgentConfig.labels:type_name -> google.protobuf.Struct - 45, // 50: mpi.v1.CommandServer.server:type_name -> mpi.v1.ServerSettings - 46, // 51: mpi.v1.CommandServer.auth:type_name -> mpi.v1.AuthSettings - 47, // 52: mpi.v1.CommandServer.tls:type_name -> mpi.v1.TLSSettings - 2, // 53: mpi.v1.CommandService.CreateConnection:input_type -> mpi.v1.CreateConnectionRequest - 8, // 54: mpi.v1.CommandService.UpdateDataPlaneStatus:input_type -> mpi.v1.UpdateDataPlaneStatusRequest - 11, // 55: mpi.v1.CommandService.UpdateDataPlaneHealth:input_type -> mpi.v1.UpdateDataPlaneHealthRequest - 13, // 56: mpi.v1.CommandService.Subscribe:input_type -> mpi.v1.DataPlaneResponse - 7, // 57: mpi.v1.CommandService.CreateConnection:output_type -> mpi.v1.CreateConnectionResponse - 9, // 58: mpi.v1.CommandService.UpdateDataPlaneStatus:output_type -> mpi.v1.UpdateDataPlaneStatusResponse - 12, // 59: mpi.v1.CommandService.UpdateDataPlaneHealth:output_type -> mpi.v1.UpdateDataPlaneHealthResponse - 14, // 60: mpi.v1.CommandService.Subscribe:output_type -> mpi.v1.ManagementPlaneRequest - 57, // [57:61] is the sub-list for method output_type - 53, // [53:57] is the sub-list for method input_type - 53, // [53:53] is the sub-list for extension type_name - 53, // [53:53] is the sub-list for extension extendee - 0, // [0:53] is the sub-list for field type_name + 40, // 47: mpi.v1.AgentConfig.metrics:type_name -> mpi.v1.MetricsServer + 41, // 48: mpi.v1.AgentConfig.file:type_name -> mpi.v1.FileServer + 45, // 49: mpi.v1.AgentConfig.labels:type_name -> google.protobuf.Struct + 39, // 50: mpi.v1.AgentConfig.auxiliary_command:type_name -> mpi.v1.AuxiliaryCommandServer + 46, // 51: mpi.v1.CommandServer.server:type_name -> mpi.v1.ServerSettings + 47, // 52: mpi.v1.CommandServer.auth:type_name -> mpi.v1.AuthSettings + 48, // 53: mpi.v1.CommandServer.tls:type_name -> mpi.v1.TLSSettings + 46, // 54: mpi.v1.AuxiliaryCommandServer.server:type_name -> mpi.v1.ServerSettings + 47, // 55: mpi.v1.AuxiliaryCommandServer.auth:type_name -> mpi.v1.AuthSettings + 48, // 56: mpi.v1.AuxiliaryCommandServer.tls:type_name -> mpi.v1.TLSSettings + 2, // 57: mpi.v1.CommandService.CreateConnection:input_type -> mpi.v1.CreateConnectionRequest + 8, // 58: mpi.v1.CommandService.UpdateDataPlaneStatus:input_type -> mpi.v1.UpdateDataPlaneStatusRequest + 11, // 59: mpi.v1.CommandService.UpdateDataPlaneHealth:input_type -> mpi.v1.UpdateDataPlaneHealthRequest + 13, // 60: mpi.v1.CommandService.Subscribe:input_type -> mpi.v1.DataPlaneResponse + 7, // 61: mpi.v1.CommandService.CreateConnection:output_type -> mpi.v1.CreateConnectionResponse + 9, // 62: mpi.v1.CommandService.UpdateDataPlaneStatus:output_type -> mpi.v1.UpdateDataPlaneStatusResponse + 12, // 63: mpi.v1.CommandService.UpdateDataPlaneHealth:output_type -> mpi.v1.UpdateDataPlaneHealthResponse + 14, // 64: mpi.v1.CommandService.Subscribe:output_type -> mpi.v1.ManagementPlaneRequest + 61, // [61:65] is the sub-list for method output_type + 57, // [57:61] is the sub-list for method input_type + 57, // [57:57] is the sub-list for extension type_name + 57, // [57:57] is the sub-list for extension extendee + 0, // [0:57] is the sub-list for field type_name } func init() { file_mpi_v1_command_proto_init() } @@ -2995,7 +3078,7 @@ func file_mpi_v1_command_proto_init() { GoPackagePath: reflect.TypeOf(x{}).PkgPath(), RawDescriptor: unsafe.Slice(unsafe.StringData(file_mpi_v1_command_proto_rawDesc), len(file_mpi_v1_command_proto_rawDesc)), NumEnums: 2, - NumMessages: 39, + NumMessages: 40, NumExtensions: 0, NumServices: 1, }, diff --git a/api/grpc/mpi/v1/command.pb.validate.go b/api/grpc/mpi/v1/command.pb.validate.go index 34825aa11..8ae81299b 100644 --- a/api/grpc/mpi/v1/command.pb.validate.go +++ b/api/grpc/mpi/v1/command.pb.validate.go @@ -5323,6 +5323,35 @@ func (m *AgentConfig) validate(all bool) error { // no validation rules for MessageBufferSize + if all { + switch v := interface{}(m.GetAuxiliaryCommand()).(type) { + case interface{ ValidateAll() error }: + if err := v.ValidateAll(); err != nil { + errors = append(errors, AgentConfigValidationError{ + field: "AuxiliaryCommand", + reason: "embedded message failed validation", + cause: err, + }) + } + case interface{ Validate() error }: + if err := v.Validate(); err != nil { + errors = append(errors, AgentConfigValidationError{ + field: "AuxiliaryCommand", + reason: "embedded message failed validation", + cause: err, + }) + } + } + } else if v, ok := interface{}(m.GetAuxiliaryCommand()).(interface{ Validate() error }); ok { + if err := v.Validate(); err != nil { + return AgentConfigValidationError{ + field: "AuxiliaryCommand", + reason: "embedded message failed validation", + cause: err, + } + } + } + if len(errors) > 0 { return AgentConfigMultiError(errors) } @@ -5587,6 +5616,195 @@ var _ interface { ErrorName() string } = CommandServerValidationError{} +// Validate checks the field values on AuxiliaryCommandServer with the rules +// defined in the proto definition for this message. If any rules are +// violated, the first error encountered is returned, or nil if there are no violations. +func (m *AuxiliaryCommandServer) Validate() error { + return m.validate(false) +} + +// ValidateAll checks the field values on AuxiliaryCommandServer with the rules +// defined in the proto definition for this message. If any rules are +// violated, the result is a list of violation errors wrapped in +// AuxiliaryCommandServerMultiError, or nil if none found. +func (m *AuxiliaryCommandServer) ValidateAll() error { + return m.validate(true) +} + +func (m *AuxiliaryCommandServer) validate(all bool) error { + if m == nil { + return nil + } + + var errors []error + + if all { + switch v := interface{}(m.GetServer()).(type) { + case interface{ ValidateAll() error }: + if err := v.ValidateAll(); err != nil { + errors = append(errors, AuxiliaryCommandServerValidationError{ + field: "Server", + reason: "embedded message failed validation", + cause: err, + }) + } + case interface{ Validate() error }: + if err := v.Validate(); err != nil { + errors = append(errors, AuxiliaryCommandServerValidationError{ + field: "Server", + reason: "embedded message failed validation", + cause: err, + }) + } + } + } else if v, ok := interface{}(m.GetServer()).(interface{ Validate() error }); ok { + if err := v.Validate(); err != nil { + return AuxiliaryCommandServerValidationError{ + field: "Server", + reason: "embedded message failed validation", + cause: err, + } + } + } + + if all { + switch v := interface{}(m.GetAuth()).(type) { + case interface{ ValidateAll() error }: + if err := v.ValidateAll(); err != nil { + errors = append(errors, AuxiliaryCommandServerValidationError{ + field: "Auth", + reason: "embedded message failed validation", + cause: err, + }) + } + case interface{ Validate() error }: + if err := v.Validate(); err != nil { + errors = append(errors, AuxiliaryCommandServerValidationError{ + field: "Auth", + reason: "embedded message failed validation", + cause: err, + }) + } + } + } else if v, ok := interface{}(m.GetAuth()).(interface{ Validate() error }); ok { + if err := v.Validate(); err != nil { + return AuxiliaryCommandServerValidationError{ + field: "Auth", + reason: "embedded message failed validation", + cause: err, + } + } + } + + if all { + switch v := interface{}(m.GetTls()).(type) { + case interface{ ValidateAll() error }: + if err := v.ValidateAll(); err != nil { + errors = append(errors, AuxiliaryCommandServerValidationError{ + field: "Tls", + reason: "embedded message failed validation", + cause: err, + }) + } + case interface{ Validate() error }: + if err := v.Validate(); err != nil { + errors = append(errors, AuxiliaryCommandServerValidationError{ + field: "Tls", + reason: "embedded message failed validation", + cause: err, + }) + } + } + } else if v, ok := interface{}(m.GetTls()).(interface{ Validate() error }); ok { + if err := v.Validate(); err != nil { + return AuxiliaryCommandServerValidationError{ + field: "Tls", + reason: "embedded message failed validation", + cause: err, + } + } + } + + if len(errors) > 0 { + return AuxiliaryCommandServerMultiError(errors) + } + + return nil +} + +// AuxiliaryCommandServerMultiError is an error wrapping multiple validation +// errors returned by AuxiliaryCommandServer.ValidateAll() if the designated +// constraints aren't met. +type AuxiliaryCommandServerMultiError []error + +// Error returns a concatenation of all the error messages it wraps. +func (m AuxiliaryCommandServerMultiError) Error() string { + msgs := make([]string, 0, len(m)) + for _, err := range m { + msgs = append(msgs, err.Error()) + } + return strings.Join(msgs, "; ") +} + +// AllErrors returns a list of validation violation errors. +func (m AuxiliaryCommandServerMultiError) AllErrors() []error { return m } + +// AuxiliaryCommandServerValidationError is the validation error returned by +// AuxiliaryCommandServer.Validate if the designated constraints aren't met. +type AuxiliaryCommandServerValidationError struct { + field string + reason string + cause error + key bool +} + +// Field function returns field value. +func (e AuxiliaryCommandServerValidationError) Field() string { return e.field } + +// Reason function returns reason value. +func (e AuxiliaryCommandServerValidationError) Reason() string { return e.reason } + +// Cause function returns cause value. +func (e AuxiliaryCommandServerValidationError) Cause() error { return e.cause } + +// Key function returns key value. +func (e AuxiliaryCommandServerValidationError) Key() bool { return e.key } + +// ErrorName returns error name. +func (e AuxiliaryCommandServerValidationError) ErrorName() string { + return "AuxiliaryCommandServerValidationError" +} + +// Error satisfies the builtin error interface +func (e AuxiliaryCommandServerValidationError) Error() string { + cause := "" + if e.cause != nil { + cause = fmt.Sprintf(" | caused by: %v", e.cause) + } + + key := "" + if e.key { + key = "key for " + } + + return fmt.Sprintf( + "invalid %sAuxiliaryCommandServer.%s: %s%s", + key, + e.field, + e.reason, + cause) +} + +var _ error = AuxiliaryCommandServerValidationError{} + +var _ interface { + Field() string + Reason() string + Key() bool + Cause() error + ErrorName() string +} = AuxiliaryCommandServerValidationError{} + // Validate checks the field values on MetricsServer with the rules defined in // the proto definition for this message. If any rules are violated, the first // error encountered is returned, or nil if there are no violations. diff --git a/api/grpc/mpi/v1/command.proto b/api/grpc/mpi/v1/command.proto index 4a0f1ffc2..f69d8ddd1 100644 --- a/api/grpc/mpi/v1/command.proto +++ b/api/grpc/mpi/v1/command.proto @@ -322,7 +322,7 @@ message NGINXRuntimeInfo { // the stub status API details APIDetails stub_status = 1; // a list of access_logs - repeated string access_logs = 2; + repeated string access_logs = 2; // a list of error_logs repeated string error_logs = 3; // List of NGINX potentially loadable modules (installed but not loaded). @@ -368,7 +368,7 @@ message NGINXAppProtectRuntimeInfo { message InstanceAction {} // This contains a series of NGINX Agent configurations -message AgentConfig { +message AgentConfig { // Command server settings CommandServer command = 1; // Metrics server settings @@ -381,6 +381,8 @@ message AgentConfig { repeated string features = 5; // Message buffer size, maximum not acknowledged messages from the subscribe perspective string message_buffer_size = 6; + // Auxiliary Command server settings + AuxiliaryCommandServer auxiliary_command = 7; } // The command server settings, associated with messaging from an external source @@ -393,6 +395,16 @@ message CommandServer { mpi.v1.TLSSettings tls = 3; } +// The auxiliary server settings, associated with messaging from an external source +message AuxiliaryCommandServer { + // Server configuration (e.g., host, port, type) + mpi.v1.ServerSettings server = 1; + // Authentication configuration (e.g., token) + mpi.v1.AuthSettings auth = 2; + // TLS configuration for secure communication + mpi.v1.TLSSettings tls = 3; +} + // The metrics settings associated with origins (sources) of the metrics and destinations (exporter) message MetricsServer {} diff --git a/docs/proto/protos.md b/docs/proto/protos.md index 6f05d530a..688a6a6c6 100644 --- a/docs/proto/protos.md +++ b/docs/proto/protos.md @@ -44,6 +44,7 @@ - [APIActionRequest](#mpi-v1-APIActionRequest) - [APIDetails](#mpi-v1-APIDetails) - [AgentConfig](#mpi-v1-AgentConfig) + - [AuxiliaryCommandServer](#mpi-v1-AuxiliaryCommandServer) - [CommandServer](#mpi-v1-CommandServer) - [CommandStatusRequest](#mpi-v1-CommandStatusRequest) - [ConfigApplyRequest](#mpi-v1-ConfigApplyRequest) @@ -697,6 +698,24 @@ This contains a series of NGINX Agent configurations | labels | [google.protobuf.Struct](#google-protobuf-Struct) | repeated | A series of key/value pairs to add more data to the NGINX Agent instance | | features | [string](#string) | repeated | A list of features that the NGINX Agent has | | message_buffer_size | [string](#string) | | Message buffer size, maximum not acknowledged messages from the subscribe perspective | +| auxiliary_command | [AuxiliaryCommandServer](#mpi-v1-AuxiliaryCommandServer) | | Auxiliary Command server settings | + + + + + + + + +### AuxiliaryCommandServer +The auxiliary server settings, associated with messaging from an external source + + +| Field | Type | Label | Description | +| ----- | ---- | ----- | ----------- | +| server | [ServerSettings](#mpi-v1-ServerSettings) | | Server configuration (e.g., host, port, type) | +| auth | [AuthSettings](#mpi-v1-AuthSettings) | | Authentication configuration (e.g., token) | +| tls | [TLSSettings](#mpi-v1-TLSSettings) | | TLS configuration for secure communication | From e07bf647f6e04948327a6a1836f9e7c79152bac5 Mon Sep 17 00:00:00 2001 From: Aphral Griffin Date: Thu, 12 Jun 2025 16:37:05 +0100 Subject: [PATCH 02/17] allow multiple management planes --- internal/command/command_plugin.go | 98 ++++++++++++------ internal/command/command_plugin_test.go | 20 ++-- internal/command/command_service.go | 1 + internal/config/config.go | 120 ++++++++++++++++++++-- internal/config/config_test.go | 18 ++++ internal/config/defaults.go | 11 ++ internal/config/flags.go | 59 +++++++---- internal/config/testdata/nginx-agent.conf | 14 +++ internal/config/types.go | 24 ++++- internal/grpc/grpc.go | 48 ++++----- internal/grpc/grpc_test.go | 6 +- internal/logger/logger.go | 15 +++ internal/plugin/plugin_manager.go | 29 +++++- internal/watcher/watcher_plugin.go | 3 +- 14 files changed, 365 insertions(+), 101 deletions(-) diff --git a/internal/command/command_plugin.go b/internal/command/command_plugin.go index 1311cd5e7..ab9b624ff 100644 --- a/internal/command/command_plugin.go +++ b/internal/command/command_plugin.go @@ -38,37 +38,45 @@ type ( } CommandPlugin struct { - messagePipe bus.MessagePipeInterface - config *config.Config - subscribeCancel context.CancelFunc - conn grpc.GrpcConnectionInterface - commandService commandService - subscribeChannel chan *mpi.ManagementPlaneRequest - subscribeMutex sync.Mutex + messagePipe bus.MessagePipeInterface + config *config.Config + subscribeCancel context.CancelFunc + conn grpc.GrpcConnectionInterface + commandService commandService + subscribeChannel chan *mpi.ManagementPlaneRequest + commandServerType string + subscribeMutex sync.Mutex } ) -func NewCommandPlugin(agentConfig *config.Config, grpcConnection grpc.GrpcConnectionInterface) *CommandPlugin { +func NewCommandPlugin(agentConfig *config.Config, grpcConnection grpc.GrpcConnectionInterface, + commandServerType string, +) *CommandPlugin { return &CommandPlugin{ - config: agentConfig, - conn: grpcConnection, - subscribeChannel: make(chan *mpi.ManagementPlaneRequest), + config: agentConfig, + conn: grpcConnection, + subscribeChannel: make(chan *mpi.ManagementPlaneRequest), + commandServerType: commandServerType, } } func (cp *CommandPlugin) Init(ctx context.Context, messagePipe bus.MessagePipeInterface) error { - slog.DebugContext(ctx, "Starting command plugin") + slog.DebugContext(ctx, "Starting command plugin", "command_server_type", cp.commandServerType) cp.messagePipe = messagePipe cp.commandService = NewCommandService(cp.conn.CommandServiceClient(), cp.config, cp.subscribeChannel) - go cp.monitorSubscribeChannel(ctx) + newCtx := context.WithValue( + ctx, + logger.ServerTypeContextKey, slog.Any(logger.ServerTypeKey, cp.commandServerType), + ) + go cp.monitorSubscribeChannel(newCtx) return nil } func (cp *CommandPlugin) Close(ctx context.Context) error { - slog.InfoContext(ctx, "Closing command plugin") + slog.InfoContext(ctx, "Closing command plugin", "command_server_type", cp.commandServerType) cp.subscribeMutex.Lock() if cp.subscribeCancel != nil { @@ -81,24 +89,31 @@ func (cp *CommandPlugin) Close(ctx context.Context) error { func (cp *CommandPlugin) Info() *bus.Info { return &bus.Info{ - Name: "command", + Name: cp.commandServerType, } } func (cp *CommandPlugin) Process(ctx context.Context, msg *bus.Message) { - switch msg.Topic { - case bus.ConnectionResetTopic: - cp.processConnectionReset(ctx, msg) - case bus.ResourceUpdateTopic: - cp.processResourceUpdate(ctx, msg) - case bus.InstanceHealthTopic: - cp.processInstanceHealth(ctx, msg) - case bus.DataPlaneHealthResponseTopic: - cp.processDataPlaneHealth(ctx, msg) - case bus.DataPlaneResponseTopic: - cp.processDataPlaneResponse(ctx, msg) - default: - slog.DebugContext(ctx, "Command plugin received unknown topic", "topic", msg.Topic) + slog.DebugContext(ctx, "Processing command", "command_server_type", logger.ServerType(ctx)) + + if logger.ServerType(ctx) == cp.commandServerType || logger.ServerType(ctx) == "" { + switch msg.Topic { + case bus.ConnectionResetTopic: + cp.processConnectionReset(ctx, msg) + case bus.ResourceUpdateTopic: + cp.processResourceUpdate(ctx, msg) + case bus.InstanceHealthTopic: + cp.processInstanceHealth(ctx, msg) + case bus.DataPlaneHealthResponseTopic: + cp.processDataPlaneHealth(ctx, msg) + case bus.DataPlaneResponseTopic: + cp.processDataPlaneResponse(ctx, msg) + default: + slog.DebugContext(ctx, "Command plugin received unknown topic", "topic", msg.Topic) + } + } else { + slog.Info("Sever type is not right ignoring message", "command_server_type", + logger.ServerType(ctx), "topic", msg.Topic) } } @@ -106,7 +121,11 @@ func (cp *CommandPlugin) processResourceUpdate(ctx context.Context, msg *bus.Mes slog.DebugContext(ctx, "Command plugin received resource update message") if resource, ok := msg.Data.(*mpi.Resource); ok { if !cp.commandService.IsConnected() { - cp.createConnection(ctx, resource) + newCtx := context.WithValue( + ctx, + logger.ServerTypeContextKey, slog.Any(logger.ServerTypeKey, cp.commandServerType), + ) + cp.createConnection(newCtx, resource) } else { statusErr := cp.commandService.UpdateDataPlaneStatus(ctx, resource) if statusErr != nil { @@ -145,13 +164,14 @@ func (cp *CommandPlugin) processDataPlaneHealth(ctx context.Context, msg *bus.Me correlationID := logger.CorrelationID(ctx) if err != nil { slog.ErrorContext(ctx, "Unable to update data plane health", "error", err) - cp.messagePipe.Process(ctx, &bus.Message{ + + cp.processDataPlaneResponse(ctx, &bus.Message{ Topic: bus.DataPlaneResponseTopic, Data: cp.createDataPlaneResponse(correlationID, mpi.CommandResponse_COMMAND_STATUS_FAILURE, "Failed to send the health status update", err.Error()), }) } - cp.messagePipe.Process(ctx, &bus.Message{ + cp.processDataPlaneResponse(ctx, &bus.Message{ Topic: bus.DataPlaneResponseTopic, Data: cp.createDataPlaneResponse(correlationID, mpi.CommandResponse_COMMAND_STATUS_OK, "Successfully sent health status update", ""), @@ -164,7 +184,8 @@ func (cp *CommandPlugin) processInstanceHealth(ctx context.Context, msg *bus.Mes if instances, ok := msg.Data.([]*mpi.InstanceHealth); ok { err := cp.commandService.UpdateDataPlaneHealth(ctx, instances) if err != nil { - slog.ErrorContext(ctx, "Unable to update data plane health", "error", err) + slog.ErrorContext(ctx, "Unable to update data plane health", "error", err, + "command_server_type", cp.commandServerType) } } } @@ -208,6 +229,7 @@ func (cp *CommandPlugin) Subscriptions() []string { } } +// nolint: revive, cyclop func (cp *CommandPlugin) monitorSubscribeChannel(ctx context.Context) { for { select { @@ -226,12 +248,24 @@ func (cp *CommandPlugin) monitorSubscribeChannel(ctx context.Context) { slog.InfoContext(ctx, "Received management plane config upload request") cp.handleConfigUploadRequest(newCtx, message) case *mpi.ManagementPlaneRequest_ConfigApplyRequest: + if cp.commandServerType != "command" { + slog.WarnContext(newCtx, "Auxiliary command server can not perform config apply", + "command_server_type", cp.commandServerType) + + return + } slog.InfoContext(ctx, "Received management plane config apply request") cp.handleConfigApplyRequest(newCtx, message) case *mpi.ManagementPlaneRequest_HealthRequest: slog.InfoContext(ctx, "Received management plane health request") cp.handleHealthRequest(newCtx) case *mpi.ManagementPlaneRequest_ActionRequest: + if cp.commandServerType != "command" { + slog.WarnContext(newCtx, "Auxiliary command server can not perform api action", + "command_server_type", cp.commandServerType) + + return + } slog.InfoContext(ctx, "Received management plane action request") cp.handleAPIActionRequest(newCtx, message) default: diff --git a/internal/command/command_plugin_test.go b/internal/command/command_plugin_test.go index d3d0c51da..9048e4873 100644 --- a/internal/command/command_plugin_test.go +++ b/internal/command/command_plugin_test.go @@ -31,15 +31,17 @@ import ( "github.com/stretchr/testify/require" ) +const serverType = "command" + func TestCommandPlugin_Info(t *testing.T) { - commandPlugin := NewCommandPlugin(types.AgentConfig(), &grpcfakes.FakeGrpcConnectionInterface{}) + commandPlugin := NewCommandPlugin(types.AgentConfig(), &grpcfakes.FakeGrpcConnectionInterface{}, serverType) info := commandPlugin.Info() assert.Equal(t, "command", info.Name) } func TestCommandPlugin_Subscriptions(t *testing.T) { - commandPlugin := NewCommandPlugin(types.AgentConfig(), &grpcfakes.FakeGrpcConnectionInterface{}) + commandPlugin := NewCommandPlugin(types.AgentConfig(), &grpcfakes.FakeGrpcConnectionInterface{}, serverType) subscriptions := commandPlugin.Subscriptions() assert.Equal( @@ -60,7 +62,7 @@ func TestCommandPlugin_Init(t *testing.T) { messagePipe := busfakes.NewFakeMessagePipe() fakeCommandService := &commandfakes.FakeCommandService{} - commandPlugin := NewCommandPlugin(types.AgentConfig(), &grpcfakes.FakeGrpcConnectionInterface{}) + commandPlugin := NewCommandPlugin(types.AgentConfig(), &grpcfakes.FakeGrpcConnectionInterface{}, serverType) err := commandPlugin.Init(ctx, messagePipe) require.NoError(t, err) @@ -79,7 +81,7 @@ func TestCommandPlugin_createConnection(t *testing.T) { commandService.CreateConnectionReturns(&mpi.CreateConnectionResponse{}, nil) messagePipe := busfakes.NewFakeMessagePipe() - commandPlugin := NewCommandPlugin(types.AgentConfig(), &grpcfakes.FakeGrpcConnectionInterface{}) + commandPlugin := NewCommandPlugin(types.AgentConfig(), &grpcfakes.FakeGrpcConnectionInterface{}, serverType) err := commandPlugin.Init(ctx, messagePipe) commandPlugin.commandService = commandService require.NoError(t, err) @@ -111,7 +113,7 @@ func TestCommandPlugin_Process(t *testing.T) { messagePipe := busfakes.NewFakeMessagePipe() fakeCommandService := &commandfakes.FakeCommandService{} - commandPlugin := NewCommandPlugin(types.AgentConfig(), &grpcfakes.FakeGrpcConnectionInterface{}) + commandPlugin := NewCommandPlugin(types.AgentConfig(), &grpcfakes.FakeGrpcConnectionInterface{}, serverType) err := commandPlugin.Init(ctx, messagePipe) require.NoError(t, err) defer commandPlugin.Close(ctx) @@ -219,7 +221,7 @@ func TestCommandPlugin_monitorSubscribeChannel(t *testing.T) { agentConfig := types.AgentConfig() agentConfig.Features = test.configFeatures - commandPlugin := NewCommandPlugin(agentConfig, &grpcfakes.FakeGrpcConnectionInterface{}) + commandPlugin := NewCommandPlugin(agentConfig, &grpcfakes.FakeGrpcConnectionInterface{}, serverType) err := commandPlugin.Init(ctx, messagePipe) require.NoError(tt, err) defer commandPlugin.Close(ctx) @@ -319,7 +321,7 @@ func TestCommandPlugin_FeatureDisabled(t *testing.T) { agentConfig.Features = test.configFeatures - commandPlugin := NewCommandPlugin(agentConfig, &grpcfakes.FakeGrpcConnectionInterface{}) + commandPlugin := NewCommandPlugin(agentConfig, &grpcfakes.FakeGrpcConnectionInterface{}, serverType) err := commandPlugin.Init(ctx, messagePipe) commandPlugin.commandService = fakeCommandService require.NoError(tt, err) @@ -344,7 +346,7 @@ func TestMonitorSubscribeChannel(t *testing.T) { logBuf := &bytes.Buffer{} stub.StubLoggerWith(logBuf) - cp := NewCommandPlugin(types.AgentConfig(), &grpcfakes.FakeGrpcConnectionInterface{}) + cp := NewCommandPlugin(types.AgentConfig(), &grpcfakes.FakeGrpcConnectionInterface{}, serverType) cp.subscribeCancel = cncl message := protos.CreateManagementPlaneRequest() @@ -383,7 +385,7 @@ func Test_createDataPlaneResponse(t *testing.T) { Error: "", }, } - commandPlugin := NewCommandPlugin(types.AgentConfig(), &grpcfakes.FakeGrpcConnectionInterface{}) + commandPlugin := NewCommandPlugin(types.AgentConfig(), &grpcfakes.FakeGrpcConnectionInterface{}, serverType) result := commandPlugin.createDataPlaneResponse(expected.GetMessageMeta().GetCorrelationId(), expected.GetCommandResponse().GetStatus(), expected.GetCommandResponse().GetMessage(), expected.GetCommandResponse().GetError()) diff --git a/internal/command/command_service.go b/internal/command/command_service.go index 5caa431d5..4f4746790 100644 --- a/internal/command/command_service.go +++ b/internal/command/command_service.go @@ -261,6 +261,7 @@ func (cs *CommandService) UpdateClient(ctx context.Context, client mpi.CommandSe cs.subscribeClientMutex.Unlock() cs.isConnected.Store(false) + // Will this have the sever type ? resp, err := cs.CreateConnection(ctx, cs.resource) if err != nil { return err diff --git a/internal/config/config.go b/internal/config/config.go index 85cdc6a71..07a64ffdd 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -121,6 +121,7 @@ func ResolveConfig() (*Config, error) { AllowedDirectories: allowedDirs, Collector: collector, Command: resolveCommand(), + AuxiliaryCommand: resolveAuxiliaryCommand(), Watchers: resolveWatchers(), Features: viperInstance.GetStringSlice(FeaturesKey), Labels: resolveLabels(), @@ -137,8 +138,9 @@ func ResolveConfig() (*Config, error) { } func checkCollectorConfiguration(collector *Collector, config *Config) { - if isOTelExporterConfigured(collector) && config.IsGrpcClientConfigured() && config.IsAuthConfigured() && - config.IsTLSConfigured() { + if isOTelExporterConfigured(collector) && config.IsCommandGrpcClientConfigured() && + config.IsCommandAuthConfigured() && + config.IsCommandTLSConfigured() { slog.Info("No collector configuration found in NGINX Agent config, command server configuration found." + " Using default collector configuration") defaultCollector(collector, config) @@ -291,6 +293,7 @@ func registerFlags() { registerCommonFlags(fs) registerCommandFlags(fs) + registerAuxiliaryCommandFlags(fs) registerCollectorFlags(fs) registerClientFlags(fs) @@ -451,6 +454,60 @@ func registerCommandFlags(fs *flag.FlagSet) { ) } +func registerAuxiliaryCommandFlags(fs *flag.FlagSet) { + fs.String( + AuxiliaryCommandServerHostKey, + DefAuxiliaryCommandServerHostKey, + "The target hostname of the auxiliary server endpoint for read only mode.", + ) + fs.Int32( + AuxiliaryCommandServerPortKey, + DefAuxiliaryCommandServerPortKey, + "The target port of the auxiliary server endpoint for read only mode.", + ) + fs.String( + AuxiliaryCommandServerTypeKey, + DefAuxiliaryCommandServerTypeKey, + "The target protocol (gRPC or HTTP) the auxiliary server endpoint for read only mode.", + ) + fs.String( + AuxiliaryCommandAuthTokenKey, + DefAuxiliaryCommandAuthTokenKey, + "The token used in the authentication handshake with the auxiliary server endpoint for read only mode.", + ) + fs.String( + AuxiliaryCommandAuthTokenPathKey, + DefAuxiliaryCommandAuthTokenPathKey, + "The path to the file containing the token used in the authentication handshake with "+ + "the auxiliary server endpoint for read only mode.", + ) + fs.String( + AuxiliaryCommandTLSCertKey, + DefAuxiliaryCommandTLSCertKey, + "The path to the certificate file to use for TLS communication with the auxiliary command server.", + ) + fs.String( + AuxiliaryCommandTLSKeyKey, + DefAuxiliaryCommandTLSKeyKey, + "The path to the certificate key file to use for TLS communication with the auxiliary command server.", + ) + fs.String( + AuxiliaryCommandTLSCaKey, + DefAuxiliaryCommandTLSCaKey, + "The path to CA certificate file to use for TLS communication with the auxiliary command server.", + ) + fs.Bool( + AuxiliaryCommandTLSSkipVerifyKey, + DefAuxiliaryCommandTLSSkipVerifyKey, + "Testing only. Skip verify controls client verification of a server's certificate chain and host name.", + ) + fs.String( + AuxiliaryCommandTLSServerNameKey, + DefAuxiliaryCommandTLServerNameKey, + "Specifies the name of the server sent in the TLS configuration.", + ) +} + func registerCollectorFlags(fs *flag.FlagSet) { fs.String( CollectorConfigPathKey, @@ -985,14 +1042,14 @@ func resolveCommand() *Command { }, } - if areAuthSettingsSet() { + if areCommandAuthSettingsSet() { command.Auth = &AuthConfig{ Token: viperInstance.GetString(CommandAuthTokenKey), TokenPath: viperInstance.GetString(CommandAuthTokenPathKey), } } - if areTLSSettingsSet() { + if areCommandTLSSettingsSet() { command.TLS = &TLSConfig{ Cert: viperInstance.GetString(CommandTLSCertKey), Key: viperInstance.GetString(CommandTLSKeyKey), @@ -1005,12 +1062,55 @@ func resolveCommand() *Command { return command } -func areAuthSettingsSet() bool { +func resolveAuxiliaryCommand() *Command { + serverType, ok := parseServerType(viperInstance.GetString(AuxiliaryCommandServerTypeKey)) + if !ok { + serverType = Grpc + slog.Error( + "Invalid value for auxiliary command server type, defaulting to gRPC server type", + "server_type", viperInstance.GetString(AuxiliaryCommandServerTypeKey), + ) + } + + auxiliary := &Command{ + Server: &ServerConfig{ + Host: viperInstance.GetString(AuxiliaryCommandServerHostKey), + Port: viperInstance.GetInt(AuxiliaryCommandServerPortKey), + Type: serverType, + }, + } + + if areAuxiliaryCommandAuthSettingsSet() { + auxiliary.Auth = &AuthConfig{ + Token: viperInstance.GetString(AuxiliaryCommandAuthTokenKey), + TokenPath: viperInstance.GetString(AuxiliaryCommandAuthTokenPathKey), + } + } + + if areAuxiliaryCommandTLSSettingsSet() { + auxiliary.TLS = &TLSConfig{ + Cert: viperInstance.GetString(AuxiliaryCommandTLSCertKey), + Key: viperInstance.GetString(AuxiliaryCommandTLSKeyKey), + Ca: viperInstance.GetString(AuxiliaryCommandTLSCaKey), + SkipVerify: viperInstance.GetBool(AuxiliaryCommandTLSSkipVerifyKey), + ServerName: viperInstance.GetString(AuxiliaryCommandTLSServerNameKey), + } + } + + return auxiliary +} + +func areCommandAuthSettingsSet() bool { return viperInstance.IsSet(CommandAuthTokenKey) || viperInstance.IsSet(CommandAuthTokenPathKey) } -func areTLSSettingsSet() bool { +func areAuxiliaryCommandAuthSettingsSet() bool { + return viperInstance.IsSet(AuxiliaryCommandAuthTokenKey) || + viperInstance.IsSet(AuxiliaryCommandAuthTokenPathKey) +} + +func areCommandTLSSettingsSet() bool { return viperInstance.IsSet(CommandTLSCertKey) || viperInstance.IsSet(CommandTLSKeyKey) || viperInstance.IsSet(CommandTLSCaKey) || @@ -1018,6 +1118,14 @@ func areTLSSettingsSet() bool { viperInstance.IsSet(CommandTLSServerNameKey) } +func areAuxiliaryCommandTLSSettingsSet() bool { + return viperInstance.IsSet(AuxiliaryCommandTLSCertKey) || + viperInstance.IsSet(AuxiliaryCommandTLSKeyKey) || + viperInstance.IsSet(AuxiliaryCommandTLSCaKey) || + viperInstance.IsSet(AuxiliaryCommandTLSSkipVerifyKey) || + viperInstance.IsSet(AuxiliaryCommandTLSServerNameKey) +} + func areHealthExtensionTLSSettingsSet() bool { return viperInstance.IsSet(CollectorExtensionsHealthTLSCertKey) || viperInstance.IsSet(CollectorExtensionsHealthTLSKeyKey) || diff --git a/internal/config/config_test.go b/internal/config/config_test.go index 6ec1f7775..f7d2f1de3 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -1082,6 +1082,24 @@ func createConfig() *Config { ServerName: "server-name", }, }, + AuxiliaryCommand: &Command{ + Server: &ServerConfig{ + Host: "second.management.plane", + Port: 9999, + Type: Grpc, + }, + Auth: &AuthConfig{ + Token: "1234", + TokenPath: "path/to/my_token", + }, + TLS: &TLSConfig{ + Cert: "some.cert", + Key: "some.key", + Ca: "some.ca", + SkipVerify: false, + ServerName: "server-name", + }, + }, Watchers: &Watchers{ InstanceWatcher: InstanceWatcher{ MonitoringFrequency: 10 * time.Second, diff --git a/internal/config/defaults.go b/internal/config/defaults.go index ddeee7a3b..f877f203a 100644 --- a/internal/config/defaults.go +++ b/internal/config/defaults.go @@ -26,6 +26,17 @@ const ( DefCommandTLSSkipVerifyKey = false DefCommandTLServerNameKey = "" + DefAuxiliaryCommandServerHostKey = "" + DefAuxiliaryCommandServerPortKey = 0 + DefAuxiliaryCommandServerTypeKey = "grpc" + DefAuxiliaryCommandAuthTokenKey = "" + DefAuxiliaryCommandAuthTokenPathKey = "" + DefAuxiliaryCommandTLSCertKey = "" + DefAuxiliaryCommandTLSKeyKey = "" + DefAuxiliaryCommandTLSCaKey = "" + DefAuxiliaryCommandTLSSkipVerifyKey = false + DefAuxiliaryCommandTLServerNameKey = "" + // Client GRPC Settings DefMaxMessageSize = 0 // 0 = unset DefMaxMessageRecieveSize = 4194304 // default 4 MB diff --git a/internal/config/flags.go b/internal/config/flags.go index 50fb461e5..d570e3b44 100644 --- a/internal/config/flags.go +++ b/internal/config/flags.go @@ -13,6 +13,7 @@ const ( AllowedDirectoriesKey = "allowed_directories" ConfigPathKey = "path" CommandRootKey = "command" + AuxiliaryCommandRootKey = "auxiliary_command" DataPlaneConfigRootKey = "data_plane_config" LabelsRootKey = "labels" LogLevelRootKey = "log" @@ -81,26 +82,44 @@ var ( CollectorLogKey = pre(CollectorRootKey) + "log" CollectorLogLevelKey = pre(CollectorLogKey) + "level" CollectorLogPathKey = pre(CollectorLogKey) + "path" - CommandAuthKey = pre(CommandRootKey) + "auth" - CommandAuthTokenKey = pre(CommandAuthKey) + "token" - CommandAuthTokenPathKey = pre(CommandAuthKey) + "tokenpath" - CommandServerHostKey = pre(CommandServerKey) + "host" - CommandServerKey = pre(CommandRootKey) + "server" - CommandServerPortKey = pre(CommandServerKey) + "port" - CommandServerTypeKey = pre(CommandServerKey) + "type" - CommandTLSKey = pre(CommandRootKey) + "tls" - CommandTLSCaKey = pre(CommandTLSKey) + "ca" - CommandTLSCertKey = pre(CommandTLSKey) + "cert" - CommandTLSKeyKey = pre(CommandTLSKey) + "key" - CommandTLSServerNameKey = pre(CommandTLSKey) + "server_name" - CommandTLSSkipVerifyKey = pre(CommandTLSKey) + "skip_verify" - LogLevelKey = pre(LogLevelRootKey) + "level" - LogPathKey = pre(LogLevelRootKey) + "path" - NginxReloadMonitoringPeriodKey = pre(DataPlaneConfigRootKey, "nginx") + "reload_monitoring_period" - NginxTreatWarningsAsErrorsKey = pre(DataPlaneConfigRootKey, "nginx") + "treat_warnings_as_errors" - NginxExcludeLogsKey = pre(DataPlaneConfigRootKey, "nginx") + "exclude_logs" - FileWatcherMonitoringFrequencyKey = pre(FileWatcherKey) + "monitoring_frequency" - NginxExcludeFilesKey = pre(FileWatcherKey) + "exclude_files" + + CommandAuthKey = pre(CommandRootKey) + "auth" + CommandAuthTokenKey = pre(CommandAuthKey) + "token" + CommandAuthTokenPathKey = pre(CommandAuthKey) + "tokenpath" + CommandServerHostKey = pre(CommandServerKey) + "host" + CommandServerKey = pre(CommandRootKey) + "server" + CommandServerPortKey = pre(CommandServerKey) + "port" + CommandServerTypeKey = pre(CommandServerKey) + "type" + CommandTLSKey = pre(CommandRootKey) + "tls" + CommandTLSCaKey = pre(CommandTLSKey) + "ca" + CommandTLSCertKey = pre(CommandTLSKey) + "cert" + CommandTLSKeyKey = pre(CommandTLSKey) + "key" + CommandTLSServerNameKey = pre(CommandTLSKey) + "server_name" + CommandTLSSkipVerifyKey = pre(CommandTLSKey) + "skip_verify" + + AuxiliaryCommandAuthKey = pre(AuxiliaryCommandRootKey) + "auth" + AuxiliaryCommandAuthTokenKey = pre(AuxiliaryCommandAuthKey) + "token" + AuxiliaryCommandAuthTokenPathKey = pre(AuxiliaryCommandAuthKey) + "tokenpath" + AuxiliaryCommandServerHostKey = pre(AuxiliaryCommandServerKey) + "host" + AuxiliaryCommandServerKey = pre(AuxiliaryCommandRootKey) + "server" + AuxiliaryCommandServerPortKey = pre(AuxiliaryCommandServerKey) + "port" + AuxiliaryCommandServerTypeKey = pre(AuxiliaryCommandServerKey) + "type" + AuxiliaryCommandTLSKey = pre(AuxiliaryCommandRootKey) + "tls" + AuxiliaryCommandTLSCaKey = pre(AuxiliaryCommandTLSKey) + "ca" + AuxiliaryCommandTLSCertKey = pre(AuxiliaryCommandTLSKey) + "cert" + AuxiliaryCommandTLSKeyKey = pre(AuxiliaryCommandTLSKey) + "key" + AuxiliaryCommandTLSServerNameKey = pre(AuxiliaryCommandTLSKey) + "server_name" + AuxiliaryCommandTLSSkipVerifyKey = pre(AuxiliaryCommandTLSKey) + "skip_verify" + + LogLevelKey = pre(LogLevelRootKey) + "level" + LogPathKey = pre(LogLevelRootKey) + "path" + + NginxReloadMonitoringPeriodKey = pre(DataPlaneConfigRootKey, "nginx") + "reload_monitoring_period" + NginxTreatWarningsAsErrorsKey = pre(DataPlaneConfigRootKey, "nginx") + "treat_warnings_as_errors" + NginxExcludeLogsKey = pre(DataPlaneConfigRootKey, "nginx") + "exclude_logs" + + FileWatcherMonitoringFrequencyKey = pre(FileWatcherKey) + "monitoring_frequency" + NginxExcludeFilesKey = pre(FileWatcherKey) + "exclude_files" ) func pre(prefixes ...string) string { diff --git a/internal/config/testdata/nginx-agent.conf b/internal/config/testdata/nginx-agent.conf index a228a4e0d..7f74dd8b6 100644 --- a/internal/config/testdata/nginx-agent.conf +++ b/internal/config/testdata/nginx-agent.conf @@ -71,6 +71,20 @@ command: ca: "some.ca" skip_verify: false server_name: "server-name" + +auxiliary_command: + server: + host: "second.management.plane" + port: 9999 + auth: + token: "1234" + tokenpath: "path/to/my_token" + tls: + cert: "some.cert" + key: "some.key" + ca: "some.ca" + skip_verify: false + server_name: "server-name" collector: config_path: "/etc/nginx-agent/nginx-agent-otelcol.yaml" diff --git a/internal/config/types.go b/internal/config/types.go index 8017987f1..004a75d5c 100644 --- a/internal/config/types.go +++ b/internal/config/types.go @@ -33,6 +33,7 @@ func parseServerType(str string) (ServerType, bool) { type ( Config struct { Command *Command `yaml:"command" mapstructure:"command"` + AuxiliaryCommand *Command `yaml:"auxiliary_command" mapstructure:"auxiliary_command"` Log *Log `yaml:"log" mapstructure:"log"` DataPlaneConfig *DataPlaneConfig `yaml:"data_plane_config" mapstructure:"data_plane_config"` Client *Client `yaml:"client" mapstructure:"client"` @@ -356,7 +357,7 @@ func (c *Config) IsDirectoryAllowed(directory string) bool { return isAllowedDir(directory, c.AllowedDirectories) } -func (c *Config) IsGrpcClientConfigured() bool { +func (c *Config) IsCommandGrpcClientConfigured() bool { return c.Command != nil && c.Command.Server != nil && c.Command.Server.Host != "" && @@ -364,15 +365,32 @@ func (c *Config) IsGrpcClientConfigured() bool { c.Command.Server.Type == Grpc } -func (c *Config) IsAuthConfigured() bool { +func (c *Config) IsAuxiliaryCommandGrpcClientConfigured() bool { + return c.AuxiliaryCommand != nil && + c.AuxiliaryCommand.Server != nil && + c.AuxiliaryCommand.Server.Host != "" && + c.AuxiliaryCommand.Server.Port != 0 && + c.AuxiliaryCommand.Server.Type == Grpc +} + +func (c *Config) IsCommandAuthConfigured() bool { return c.Command.Auth != nil && (c.Command.Auth.Token != "" || c.Command.Auth.TokenPath != "") } -func (c *Config) IsTLSConfigured() bool { +func (c *Config) IsAuxiliaryCommandAuthConfigured() bool { + return c.AuxiliaryCommand.Auth != nil && + (c.AuxiliaryCommand.Auth.Token != "" || c.AuxiliaryCommand.Auth.TokenPath != "") +} + +func (c *Config) IsCommandTLSConfigured() bool { return c.Command.TLS != nil } +func (c *Config) IsAuxiliaryCommandTLSConfigured() bool { + return c.AuxiliaryCommand.TLS != nil +} + func (c *Config) IsFeatureEnabled(feature string) bool { for _, enabledFeature := range c.Features { if enabledFeature == feature { diff --git a/internal/grpc/grpc.go b/internal/grpc/grpc.go index 8d23e0c13..a36c146df 100644 --- a/internal/grpc/grpc.go +++ b/internal/grpc/grpc.go @@ -45,9 +45,9 @@ type ( } GrpcConnection struct { - config *config.Config - conn *grpc.ClientConn - mutex sync.Mutex + commandConfig *config.Command + conn *grpc.ClientConn + mutex sync.Mutex } wrappedStream struct { @@ -69,18 +69,20 @@ var ( ) // nolint: ireturn -func NewGrpcConnection(ctx context.Context, agentConfig *config.Config) (*GrpcConnection, error) { - if agentConfig == nil || agentConfig.Command.Server.Type != config.Grpc { +func NewGrpcConnection(ctx context.Context, agentConfig *config.Config, + commandConfig *config.Command, +) (*GrpcConnection, error) { + if commandConfig == nil || commandConfig.Server.Type != config.Grpc { return nil, errors.New("invalid command server settings") } grpcConnection := &GrpcConnection{ - config: agentConfig, + commandConfig: commandConfig, } serverAddr := net.JoinHostPort( - agentConfig.Command.Server.Host, - fmt.Sprint(agentConfig.Command.Server.Port), + commandConfig.Server.Host, + fmt.Sprint(commandConfig.Server.Port), ) slog.InfoContext(ctx, "Dialing grpc server", "server_addr", serverAddr) @@ -90,7 +92,7 @@ func NewGrpcConnection(ctx context.Context, agentConfig *config.Config) (*GrpcCo var err error grpcConnection.mutex.Lock() - grpcConnection.conn, err = grpc.NewClient(serverAddr, DialOptions(agentConfig, resourceID)...) + grpcConnection.conn, err = grpc.NewClient(serverAddr, DialOptions(agentConfig, commandConfig, resourceID)...) grpcConnection.mutex.Unlock() if err != nil { return nil, err @@ -160,7 +162,7 @@ func (w *wrappedStream) SendMsg(message any) error { return w.ClientStream.SendMsg(message) } -func DialOptions(agentConfig *config.Config, resourceID string) []grpc.DialOption { +func DialOptions(agentConfig *config.Config, commandConfig *config.Command, resourceID string) []grpc.DialOption { streamClientInterceptors := []grpc.StreamClientInterceptor{grpcRetry.StreamClientInterceptor()} unaryClientInterceptors := []grpc.UnaryClientInterceptor{grpcRetry.UnaryClientInterceptor()} @@ -211,17 +213,17 @@ func DialOptions(agentConfig *config.Config, resourceID string) []grpc.DialOptio opts = append(opts, sendRecOpts...) - opts, skipToken := addTransportCredentials(agentConfig, opts) + opts, skipToken := addTransportCredentials(commandConfig, opts) - if agentConfig.Command.Auth != nil && !skipToken { - opts = addPerRPCCredentials(agentConfig, resourceID, opts) + if commandConfig.Auth != nil && !skipToken { + opts = addPerRPCCredentials(commandConfig, resourceID, opts) } return opts } -func addTransportCredentials(agentConfig *config.Config, opts []grpc.DialOption) ([]grpc.DialOption, bool) { - transportCredentials, err := transportCredentials(agentConfig) +func addTransportCredentials(commandConfig *config.Command, opts []grpc.DialOption) ([]grpc.DialOption, bool) { + transportCredentials, err := transportCredentials(commandConfig) if err != nil { slog.Error("Unable to add transport credentials to gRPC dial options, adding "+ "default transport credentials", "error", err) @@ -239,12 +241,12 @@ func addTransportCredentials(agentConfig *config.Config, opts []grpc.DialOption) return opts, false } -func addPerRPCCredentials(agentConfig *config.Config, resourceID string, opts []grpc.DialOption) []grpc.DialOption { - token := agentConfig.Command.Auth.Token +func addPerRPCCredentials(commandConfig *config.Command, resourceID string, opts []grpc.DialOption) []grpc.DialOption { + token := commandConfig.Auth.Token - if agentConfig.Command.Auth.TokenPath != "" { - slog.Debug("Reading token from file", "path", agentConfig.Command.Auth.TokenPath) - tk, err := file.ReadFromFile(agentConfig.Command.Auth.TokenPath) + if commandConfig.Auth.TokenPath != "" { + slog.Debug("Reading token from file", "path", commandConfig.Auth.TokenPath) + tk, err := file.ReadFromFile(commandConfig.Auth.TokenPath) if err == nil { token = tk } else { @@ -359,11 +361,11 @@ func validateMessage(validator protovalidate.Validator, message any) error { return nil } -func transportCredentials(agentConfig *config.Config) (credentials.TransportCredentials, error) { - if agentConfig.Command.TLS == nil { +func transportCredentials(commandConfig *config.Command) (credentials.TransportCredentials, error) { + if commandConfig.TLS == nil { return defaultCredentials, nil } - tlsConfig, err := tlsConfigForCredentials(agentConfig.Command.TLS) + tlsConfig, err := tlsConfigForCredentials(commandConfig.TLS) if err != nil { return nil, err } diff --git a/internal/grpc/grpc_test.go b/internal/grpc/grpc_test.go index 0270421d7..2da040470 100644 --- a/internal/grpc/grpc_test.go +++ b/internal/grpc/grpc_test.go @@ -64,7 +64,7 @@ func (z TestError) Error() string { func Test_GrpcConnection(t *testing.T) { ctx := context.Background() - conn, err := NewGrpcConnection(ctx, types.AgentConfig()) + conn, err := NewGrpcConnection(ctx, types.AgentConfig(), types.AgentConfig().Command) require.NoError(t, err) assert.NotNil(t, conn) @@ -185,7 +185,7 @@ func Test_GetDialOptions(t *testing.T) { test.agentConfig.Command.TLS.Ca = fmt.Sprintf("%s%s%s", tmpDir, pathSeparator, caFileName) } - options := DialOptions(test.agentConfig, "123") + options := DialOptions(test.agentConfig, test.agentConfig.Command, "123") assert.NotNil(tt, options) assert.Len(tt, options, test.expected) }) @@ -389,7 +389,7 @@ func Test_transportCredentials(t *testing.T) { } for name, tt := range tests { t.Run(name, func(t *testing.T) { - got, err := transportCredentials(tt.conf) + got, err := transportCredentials(tt.conf.Command) if tt.wantErr { require.Error(t, err, "transportCredentials(%v)", tt.conf) diff --git a/internal/logger/logger.go b/internal/logger/logger.go index ea5606728..4f903ea02 100644 --- a/internal/logger/logger.go +++ b/internal/logger/logger.go @@ -22,6 +22,7 @@ const ( filePermission = 0o600 CorrelationIDKey = "correlation_id" + ServerTypeKey = "server_type" ) var ( @@ -33,6 +34,7 @@ var ( } CorrelationIDContextKey = contextKey(CorrelationIDKey) + ServerTypeContextKey = contextKey(ServerTypeKey) ) type ( @@ -159,3 +161,16 @@ func CorrelationIDAttr(ctx context.Context) slog.Attr { return value } + +func ServerType(ctx context.Context) string { + return ServerTypeAttr(ctx).Value.String() +} + +func ServerTypeAttr(ctx context.Context) slog.Attr { + value, ok := ctx.Value(ServerTypeContextKey).(slog.Attr) + if !ok { + return slog.Any(ServerTypeKey, "") + } + + return value +} diff --git a/internal/plugin/plugin_manager.go b/internal/plugin/plugin_manager.go index 6a53a9c85..e81aa2101 100644 --- a/internal/plugin/plugin_manager.go +++ b/internal/plugin/plugin_manager.go @@ -27,6 +27,7 @@ func LoadPlugins(ctx context.Context, agentConfig *config.Config) []bus.Plugin { plugins = addResourcePlugin(plugins, agentConfig) plugins = addCommandAndFilePlugins(ctx, plugins, agentConfig) + plugins = addAuxiliaryCommandAndFilePlugins(ctx, plugins, agentConfig) plugins = addCollectorPlugin(ctx, agentConfig, plugins) plugins = addWatcherPlugin(plugins, agentConfig) @@ -41,12 +42,12 @@ func addResourcePlugin(plugins []bus.Plugin, agentConfig *config.Config) []bus.P } func addCommandAndFilePlugins(ctx context.Context, plugins []bus.Plugin, agentConfig *config.Config) []bus.Plugin { - if agentConfig.IsGrpcClientConfigured() { - grpcConnection, err := grpc.NewGrpcConnection(ctx, agentConfig) + if agentConfig.IsCommandGrpcClientConfigured() { + grpcConnection, err := grpc.NewGrpcConnection(ctx, agentConfig, agentConfig.Command) if err != nil { - slog.WarnContext(ctx, "Failed to create gRPC connection", "error", err) + slog.WarnContext(ctx, "Failed to create gRPC connection for command server", "error", err) } else { - commandPlugin := command.NewCommandPlugin(agentConfig, grpcConnection) + commandPlugin := command.NewCommandPlugin(agentConfig, grpcConnection, "command") plugins = append(plugins, commandPlugin) filePlugin := file.NewFilePlugin(agentConfig, grpcConnection) plugins = append(plugins, filePlugin) @@ -59,6 +60,26 @@ func addCommandAndFilePlugins(ctx context.Context, plugins []bus.Plugin, agentCo return plugins } +func addAuxiliaryCommandAndFilePlugins(ctx context.Context, plugins []bus.Plugin, + agentConfig *config.Config, +) []bus.Plugin { + if agentConfig.IsAuxiliaryCommandGrpcClientConfigured() { + auxGRPCConnection, err := grpc.NewGrpcConnection(ctx, agentConfig, agentConfig.AuxiliaryCommand) + if err != nil { + slog.WarnContext(ctx, "Failed to create gRPC connection for auxiliary command server", "error", err) + } else { + auxCommandPlugin := command.NewCommandPlugin(agentConfig, auxGRPCConnection, "auxiliary") + plugins = append(plugins, auxCommandPlugin) + // Followup PR to add read plugin eventually + } + } else { + slog.InfoContext(ctx, "Agent is not connected to an auxiliary management plane. "+ + "Configure a auxiliary command server to establish a connection.") + } + + return plugins +} + func addCollectorPlugin(ctx context.Context, agentConfig *config.Config, plugins []bus.Plugin) []bus.Plugin { if !agentConfig.IsFeatureEnabled(pkg.FeatureMetrics) { slog.WarnContext(ctx, "Metrics feature disabled, no metrics will be collected", diff --git a/internal/watcher/watcher_plugin.go b/internal/watcher/watcher_plugin.go index a516c0b69..bbb288edf 100644 --- a/internal/watcher/watcher_plugin.go +++ b/internal/watcher/watcher_plugin.go @@ -252,7 +252,8 @@ func (w *Watcher) handleCredentialUpdate(ctx context.Context) { slog.DebugContext(ctx, "Watcher plugin received credential update message") w.watcherMutex.Lock() - conn, err := grpc.NewGrpcConnection(ctx, w.agentConfig) + // This will be changed/moved during the credential watcher PR + conn, err := grpc.NewGrpcConnection(ctx, w.agentConfig, w.agentConfig.Command) if err != nil { slog.ErrorContext(ctx, "Unable to create new grpc connection", "error", err) w.watcherMutex.Unlock() From a6b6e52a89f1ef5d9814fb1147b33c090b059a53 Mon Sep 17 00:00:00 2001 From: Aphral Griffin Date: Mon, 16 Jun 2025 14:52:51 +0100 Subject: [PATCH 03/17] start of file plugins --- internal/file/file_manager_service.go | 576 +++--------------- internal/file/file_manager_service_test.go | 141 ----- internal/file/file_operator.go | 35 ++ internal/file/file_plugin.go | 53 +- internal/file/file_plugin_test.go | 17 +- internal/file/file_service_operator.go | 512 ++++++++++++++++ internal/file/file_service_operator_test.go | 163 +++++ .../fake_file_manager_service_interface.go | 280 ++++----- internal/file/filefakes/fake_file_operator.go | 79 +++ internal/file/read_only_plugin.go | 158 +++++ internal/file/read_only_plugin_test.go | 247 ++++++++ internal/plugin/plugin_manager.go | 3 +- internal/plugin/plugin_manager_test.go | 9 + 13 files changed, 1402 insertions(+), 871 deletions(-) create mode 100644 internal/file/file_service_operator.go create mode 100644 internal/file/file_service_operator_test.go create mode 100644 internal/file/read_only_plugin.go create mode 100644 internal/file/read_only_plugin_test.go diff --git a/internal/file/file_manager_service.go b/internal/file/file_manager_service.go index acbab1f29..90c5a0767 100644 --- a/internal/file/file_manager_service.go +++ b/internal/file/file_manager_service.go @@ -12,28 +12,16 @@ import ( "errors" "fmt" "log/slog" - "maps" - "math" "os" - "slices" "sync" - "sync/atomic" "google.golang.org/grpc" "github.com/nginx/agent/v3/internal/model" - "github.com/cenkalti/backoff/v4" - mpi "github.com/nginx/agent/v3/api/grpc/mpi/v1" "github.com/nginx/agent/v3/internal/config" - internalgrpc "github.com/nginx/agent/v3/internal/grpc" - "github.com/nginx/agent/v3/internal/logger" "github.com/nginx/agent/v3/pkg/files" - "github.com/nginx/agent/v3/pkg/id" - "google.golang.org/protobuf/types/known/timestamppb" - - backoffHelpers "github.com/nginx/agent/v3/internal/backoff" ) //go:generate go run github.com/maxbrunsfeld/counterfeiter/v6@v6.8.1 -generate @@ -64,15 +52,30 @@ type ( reader *bufio.Reader, chunkID uint32, ) (mpi.FileDataChunk_Content, error) + WriteManifestFile(updatedFiles map[string]*model.ManifestFile, + manifestDir, manifestPath string) (writeError error) } - fileManagerServiceInterface interface { + fileServiceOperatorInterface interface { + File(ctx context.Context, file *mpi.File, fileActions map[string]*model.FileCache) error UpdateOverview(ctx context.Context, instanceID string, filesToUpdate []*mpi.File, iteration int) error + ChunkedFile(ctx context.Context, file *mpi.File) error + IsConnected() bool + UpdateFile( + ctx context.Context, + instanceID string, + fileToUpdate *mpi.File, + ) error + SetIsConnected(isConnected bool) + } + + fileManagerServiceInterface interface { ConfigApply(ctx context.Context, configApplyRequest *mpi.ConfigApplyRequest) (writeStatus model.WriteStatus, err error) Rollback(ctx context.Context, instanceID string) error - UpdateFile(ctx context.Context, instanceID string, fileToUpdate *mpi.File) error ClearCache() + ConfigUpload(ctx context.Context, configUploadRequest *mpi.ConfigUploadRequest) error + ConfigUpdate(ctx context.Context, nginxConfigContext *model.NginxConfigContext) UpdateCurrentFilesOnDisk(ctx context.Context, updateFiles map[string]*mpi.File, referenced bool) error DetermineFileActions(currentFiles map[string]*mpi.File, modifiedFiles map[string]*model.FileCache) ( map[string]*model.FileCache, map[string][]byte, error) @@ -82,10 +85,9 @@ type ( ) type FileManagerService struct { - fileServiceClient mpi.FileServiceClient - agentConfig *config.Config - isConnected *atomic.Bool - fileOperator fileOperator + agentConfig *config.Config + fileOperator fileOperator + fileServiceOperator fileServiceOperatorInterface // map of files and the actions performed on them during config apply fileActions map[string]*model.FileCache // key is file path // map of the contents of files which have been updated or deleted during config apply, used during rollback @@ -98,397 +100,24 @@ type FileManagerService struct { } func NewFileManagerService(fileServiceClient mpi.FileServiceClient, agentConfig *config.Config) *FileManagerService { - isConnected := &atomic.Bool{} - isConnected.Store(false) - return &FileManagerService{ - fileServiceClient: fileServiceClient, agentConfig: agentConfig, fileOperator: NewFileOperator(), + fileServiceOperator: NewFileServiceOperator(agentConfig, fileServiceClient), fileActions: make(map[string]*model.FileCache), rollbackFileContents: make(map[string][]byte), currentFilesOnDisk: make(map[string]*mpi.File), previousManifestFiles: make(map[string]*model.ManifestFile), manifestFilePath: agentConfig.ManifestDir + "/manifest.json", - isConnected: isConnected, - } -} - -func (fms *FileManagerService) UpdateOverview( - ctx context.Context, - instanceID string, - filesToUpdate []*mpi.File, - iteration int, -) error { - correlationID := logger.CorrelationID(ctx) - - // error case for the UpdateOverview attempts - if iteration > maxAttempts { - return errors.New("too many UpdateOverview attempts") - } - - newCtx, correlationID := fms.setupIdentifiers(ctx, iteration) - - request := &mpi.UpdateOverviewRequest{ - MessageMeta: &mpi.MessageMeta{ - MessageId: id.GenerateMessageID(), - CorrelationId: correlationID, - Timestamp: timestamppb.Now(), - }, - Overview: &mpi.FileOverview{ - Files: filesToUpdate, - ConfigVersion: &mpi.ConfigVersion{ - InstanceId: instanceID, - Version: files.GenerateConfigVersion(filesToUpdate), - }, - }, - } - - backOffCtx, backoffCancel := context.WithTimeout(newCtx, fms.agentConfig.Client.Backoff.MaxElapsedTime) - defer backoffCancel() - - sendUpdateOverview := func() (*mpi.UpdateOverviewResponse, error) { - if fms.fileServiceClient == nil { - return nil, errors.New("file service client is not initialized") - } - - if !fms.isConnected.Load() { - return nil, errors.New("CreateConnection rpc has not being called yet") - } - - slog.DebugContext(newCtx, "Sending update overview request", - "instance_id", request.GetOverview().GetConfigVersion().GetInstanceId(), - "request", request, "parent_correlation_id", correlationID, - ) - - response, updateError := fms.fileServiceClient.UpdateOverview(newCtx, request) - - validatedError := internalgrpc.ValidateGrpcError(updateError) - - if validatedError != nil { - slog.ErrorContext(newCtx, "Failed to send update overview", "error", validatedError) - - return nil, validatedError - } - - return response, nil } - - backoffSettings := fms.agentConfig.Client.Backoff - response, err := backoff.RetryWithData( - sendUpdateOverview, - backoffHelpers.Context(backOffCtx, backoffSettings), - ) - if err != nil { - return err - } - - slog.DebugContext(newCtx, "UpdateOverview response", "response", response) - - if response.GetOverview() == nil { - slog.Debug("UpdateOverview response is empty") - return nil - } - delta := files.ConvertToMapOfFiles(response.GetOverview().GetFiles()) - - if len(delta) != 0 { - return fms.updateFiles(ctx, delta, instanceID, iteration) - } - - return err -} - -func (fms *FileManagerService) setupIdentifiers(ctx context.Context, iteration int) (context.Context, string) { - correlationID := logger.CorrelationID(ctx) - var requestCorrelationID slog.Attr - - if iteration == 0 { - requestCorrelationID = logger.GenerateCorrelationID() - } else { - requestCorrelationID = logger.CorrelationIDAttr(ctx) - } - - newCtx := context.WithValue(ctx, logger.CorrelationIDContextKey, requestCorrelationID) - - return newCtx, correlationID -} - -func (fms *FileManagerService) updateFiles( - ctx context.Context, - delta map[string]*mpi.File, - instanceID string, - iteration int, -) error { - diffFiles := slices.Collect(maps.Values(delta)) - - for _, file := range diffFiles { - updateErr := fms.UpdateFile(ctx, instanceID, file) - if updateErr != nil { - return updateErr - } - } - - iteration++ - slog.Info("Updating file overview after file updates", "attempt_number", iteration) - - return fms.UpdateOverview(ctx, instanceID, diffFiles, iteration) -} - -func (fms *FileManagerService) UpdateFile( - ctx context.Context, - instanceID string, - fileToUpdate *mpi.File, -) error { - slog.InfoContext(ctx, "Updating file", "file_name", fileToUpdate.GetFileMeta().GetName(), "instance_id", instanceID) - - slog.DebugContext(ctx, "Checking file size", - "file_size", fileToUpdate.GetFileMeta().GetSize(), - "max_file_size", int64(fms.agentConfig.Client.Grpc.MaxFileSize), - ) - - if fileToUpdate.GetFileMeta().GetSize() <= int64(fms.agentConfig.Client.Grpc.MaxFileSize) { - return fms.sendUpdateFileRequest(ctx, fileToUpdate) - } - - return fms.sendUpdateFileStream(ctx, fileToUpdate, fms.agentConfig.Client.Grpc.FileChunkSize) -} - -func (fms *FileManagerService) sendUpdateFileRequest( - ctx context.Context, - fileToUpdate *mpi.File, -) error { - messageMeta := &mpi.MessageMeta{ - MessageId: id.GenerateMessageID(), - CorrelationId: logger.CorrelationID(ctx), - Timestamp: timestamppb.Now(), - } - - contents, err := os.ReadFile(fileToUpdate.GetFileMeta().GetName()) - if err != nil { - return err - } - - request := &mpi.UpdateFileRequest{ - File: fileToUpdate, - Contents: &mpi.FileContents{ - Contents: contents, - }, - MessageMeta: messageMeta, - } - - backOffCtx, backoffCancel := context.WithTimeout(ctx, fms.agentConfig.Client.Backoff.MaxElapsedTime) - defer backoffCancel() - - sendUpdateFile := func() (*mpi.UpdateFileResponse, error) { - slog.DebugContext(ctx, "Sending update file request", "request_file", request.GetFile(), - "request_message_meta", request.GetMessageMeta()) - if fms.fileServiceClient == nil { - return nil, errors.New("file service client is not initialized") - } - - if !fms.isConnected.Load() { - return nil, errors.New("CreateConnection rpc has not being called yet") - } - - response, updateError := fms.fileServiceClient.UpdateFile(ctx, request) - - validatedError := internalgrpc.ValidateGrpcError(updateError) - - if validatedError != nil { - slog.ErrorContext(ctx, "Failed to send update file", "error", validatedError) - - return nil, validatedError - } - - return response, nil - } - - response, err := backoff.RetryWithData( - sendUpdateFile, - backoffHelpers.Context(backOffCtx, fms.agentConfig.Client.Backoff), - ) - if err != nil { - return err - } - - slog.DebugContext(ctx, "UpdateFile response", "response", response) - - return nil -} - -func (fms *FileManagerService) sendUpdateFileStream( - ctx context.Context, - fileToUpdate *mpi.File, - chunkSize uint32, -) error { - if chunkSize == 0 { - return fmt.Errorf("file chunk size must be greater than zero") - } - - updateFileStreamClient, err := fms.fileServiceClient.UpdateFileStream(ctx) - if err != nil { - return err - } - - err = fms.sendUpdateFileStreamHeader(ctx, fileToUpdate, chunkSize, updateFileStreamClient) - if err != nil { - return err - } - - return fms.sendFileUpdateStreamChunks(ctx, fileToUpdate, chunkSize, updateFileStreamClient) -} - -func (fms *FileManagerService) sendUpdateFileStreamHeader( - ctx context.Context, - fileToUpdate *mpi.File, - chunkSize uint32, - updateFileStreamClient grpc.ClientStreamingClient[mpi.FileDataChunk, mpi.UpdateFileResponse], -) error { - messageMeta := &mpi.MessageMeta{ - MessageId: id.GenerateMessageID(), - CorrelationId: logger.CorrelationID(ctx), - Timestamp: timestamppb.Now(), - } - - numberOfChunks := uint32(math.Ceil(float64(fileToUpdate.GetFileMeta().GetSize()) / float64(chunkSize))) - - header := mpi.FileDataChunk_Header{ - Header: &mpi.FileDataChunkHeader{ - FileMeta: fileToUpdate.GetFileMeta(), - Chunks: numberOfChunks, - ChunkSize: chunkSize, - }, - } - - backOffCtx, backoffCancel := context.WithTimeout(ctx, fms.agentConfig.Client.Backoff.MaxElapsedTime) - defer backoffCancel() - - sendUpdateFileHeader := func() error { - slog.DebugContext(ctx, "Sending update file stream header", "header", header) - if fms.fileServiceClient == nil { - return errors.New("file service client is not initialized") - } - - if !fms.isConnected.Load() { - return errors.New("CreateConnection rpc has not being called yet") - } - - err := updateFileStreamClient.Send( - &mpi.FileDataChunk{ - Meta: messageMeta, - Chunk: &header, - }, - ) - - validatedError := internalgrpc.ValidateGrpcError(err) - - if validatedError != nil { - slog.ErrorContext(ctx, "Failed to send update file stream header", "error", validatedError) - - return validatedError - } - - return nil - } - - return backoff.Retry(sendUpdateFileHeader, backoffHelpers.Context(backOffCtx, fms.agentConfig.Client.Backoff)) -} - -func (fms *FileManagerService) sendFileUpdateStreamChunks( - ctx context.Context, - fileToUpdate *mpi.File, - chunkSize uint32, - updateFileStreamClient grpc.ClientStreamingClient[mpi.FileDataChunk, mpi.UpdateFileResponse], -) error { - f, err := os.Open(fileToUpdate.GetFileMeta().GetName()) - defer func() { - closeError := f.Close() - if closeError != nil { - slog.WarnContext( - ctx, "Failed to close file", - "file", fileToUpdate.GetFileMeta().GetName(), - "error", closeError, - ) - } - }() - if err != nil { - return err - } - - var chunkID uint32 - - reader := bufio.NewReader(f) - for { - chunk, readChunkError := fms.fileOperator.ReadChunk(ctx, chunkSize, reader, chunkID) - if readChunkError != nil { - return readChunkError - } - if chunk.Content == nil { - break - } - - sendError := fms.sendFileUpdateStreamChunk(ctx, chunk, updateFileStreamClient) - if sendError != nil { - return sendError - } - - chunkID++ - } - - return nil -} - -func (fms *FileManagerService) sendFileUpdateStreamChunk( - ctx context.Context, - chunk mpi.FileDataChunk_Content, - updateFileStreamClient grpc.ClientStreamingClient[mpi.FileDataChunk, mpi.UpdateFileResponse], -) error { - messageMeta := &mpi.MessageMeta{ - MessageId: id.GenerateMessageID(), - CorrelationId: logger.CorrelationID(ctx), - Timestamp: timestamppb.Now(), - } - - backOffCtx, backoffCancel := context.WithTimeout(ctx, fms.agentConfig.Client.Backoff.MaxElapsedTime) - defer backoffCancel() - - sendUpdateFileChunk := func() error { - slog.DebugContext(ctx, "Sending update file stream chunk", "chunk_id", chunk.Content.GetChunkId()) - if fms.fileServiceClient == nil { - return errors.New("file service client is not initialized") - } - - if !fms.isConnected.Load() { - return errors.New("CreateConnection rpc has not being called yet") - } - - err := updateFileStreamClient.Send( - &mpi.FileDataChunk{ - Meta: messageMeta, - Chunk: &chunk, - }, - ) - - validatedError := internalgrpc.ValidateGrpcError(err) - - if validatedError != nil { - slog.ErrorContext(ctx, "Failed to send update file stream chunk", "error", validatedError) - - return validatedError - } - - return nil - } - - return backoff.Retry(sendUpdateFileChunk, backoffHelpers.Context(backOffCtx, fms.agentConfig.Client.Backoff)) } func (fms *FileManagerService) IsConnected() bool { - return fms.isConnected.Load() + return fms.fileServiceOperator.IsConnected() } func (fms *FileManagerService) SetIsConnected(isConnected bool) { - fms.isConnected.Store(isConnected) + fms.fileServiceOperator.SetIsConnected(isConnected) } func (fms *FileManagerService) ConfigApply(ctx context.Context, @@ -573,7 +202,8 @@ func (fms *FileManagerService) Rollback(ctx context.Context, instanceID string) } } - manifestFileErr := fms.writeManifestFile(fms.previousManifestFiles) + manifestFileErr := fms.fileOperator.WriteManifestFile(fms.previousManifestFiles, + fms.agentConfig.ManifestDir, fms.manifestFilePath) if manifestFileErr != nil { return manifestFileErr } @@ -608,102 +238,70 @@ func (fms *FileManagerService) executeFileActions(ctx context.Context) error { func (fms *FileManagerService) fileUpdate(ctx context.Context, file *mpi.File) error { if file.GetFileMeta().GetSize() <= int64(fms.agentConfig.Client.Grpc.MaxFileSize) { - return fms.file(ctx, file) + return fms.fileServiceOperator.File(ctx, file, fms.fileActions) } - return fms.chunkedFile(ctx, file) + return fms.fileServiceOperator.ChunkedFile(ctx, file) } -func (fms *FileManagerService) file(ctx context.Context, file *mpi.File) error { - slog.DebugContext(ctx, "Getting file", "file", file.GetFileMeta().GetName()) - - backOffCtx, backoffCancel := context.WithTimeout(ctx, fms.agentConfig.Client.Backoff.MaxElapsedTime) - defer backoffCancel() - - getFile := func() (*mpi.GetFileResponse, error) { - return fms.fileServiceClient.GetFile(ctx, &mpi.GetFileRequest{ - MessageMeta: &mpi.MessageMeta{ - MessageId: id.GenerateMessageID(), - CorrelationId: logger.CorrelationID(ctx), - Timestamp: timestamppb.Now(), - }, - FileMeta: file.GetFileMeta(), - }) - } - - getFileResp, getFileErr := backoff.RetryWithData( - getFile, - backoffHelpers.Context(backOffCtx, fms.agentConfig.Client.Backoff), - ) - - if getFileErr != nil { - return fmt.Errorf("error getting file data for %s: %w", file.GetFileMeta(), getFileErr) - } - - if writeErr := fms.fileOperator.Write(ctx, getFileResp.GetContents().GetContents(), - file.GetFileMeta()); writeErr != nil { - return writeErr +func (fms *FileManagerService) checkAllowedDirectory(checkFiles []*mpi.File) error { + for _, file := range checkFiles { + allowed := fms.agentConfig.IsDirectoryAllowed(file.GetFileMeta().GetName()) + if !allowed { + return fmt.Errorf("file not in allowed directories %s", file.GetFileMeta().GetName()) + } } - return fms.validateFileHash(file.GetFileMeta().GetName()) + return nil } -func (fms *FileManagerService) chunkedFile(ctx context.Context, file *mpi.File) error { - slog.DebugContext(ctx, "Getting chunked file", "file", file.GetFileMeta().GetName()) - - stream, err := fms.fileServiceClient.GetFileStream(ctx, &mpi.GetFileRequest{ - MessageMeta: &mpi.MessageMeta{ - MessageId: id.GenerateMessageID(), - CorrelationId: logger.CorrelationID(ctx), - Timestamp: timestamppb.Now(), - }, - FileMeta: file.GetFileMeta(), - }) - if err != nil { - return fmt.Errorf("error getting file stream for %s: %w", file.GetFileMeta().GetName(), err) - } - - // Get header chunk first - headerChunk, recvHeaderChunkError := stream.Recv() - if recvHeaderChunkError != nil { - return recvHeaderChunkError - } - - slog.DebugContext(ctx, "File header chunk received", "header_chunk", headerChunk) - - header := headerChunk.GetHeader() - - writeChunkedFileError := fms.fileOperator.WriteChunkedFile(ctx, file, header, stream) - if writeChunkedFileError != nil { - return writeChunkedFileError +func (fms *FileManagerService) ConfigUpdate(ctx context.Context, + nginxConfigContext *model.NginxConfigContext, +) { + updateError := fms.UpdateCurrentFilesOnDisk( + ctx, + files.ConvertToMapOfFiles(nginxConfigContext.Files), + true, + ) + if updateError != nil { + slog.ErrorContext(ctx, "Unable to update current files on disk", "error", updateError) } - return nil -} - -func (fms *FileManagerService) validateFileHash(filePath string) error { - content, err := os.ReadFile(filePath) + slog.InfoContext(ctx, "Updating overview after nginx config update") + err := fms.fileServiceOperator.UpdateOverview(ctx, nginxConfigContext.InstanceID, nginxConfigContext.Files, 0) if err != nil { - return err + slog.ErrorContext( + ctx, + "Failed to update file overview", + "instance_id", nginxConfigContext.InstanceID, + "error", err, + ) } - fileHash := files.GenerateHash(content) +} - if fileHash != fms.fileActions[filePath].File.GetFileMeta().GetHash() { - return fmt.Errorf("error writing file, file hash does not match for file %s", filePath) - } +func (fms *FileManagerService) ConfigUpload(ctx context.Context, configUploadRequest *mpi.ConfigUploadRequest) error { + var updatingFilesError error - return nil -} + for _, file := range configUploadRequest.GetOverview().GetFiles() { + err := fms.fileServiceOperator.UpdateFile( + ctx, + configUploadRequest.GetOverview().GetConfigVersion().GetInstanceId(), + file, + ) + if err != nil { + slog.ErrorContext( + ctx, + "Failed to update file", + "instance_id", configUploadRequest.GetOverview().GetConfigVersion().GetInstanceId(), + "file_name", file.GetFileMeta().GetName(), + "error", err, + ) -func (fms *FileManagerService) checkAllowedDirectory(checkFiles []*mpi.File) error { - for _, file := range checkFiles { - allowed := fms.agentConfig.IsDirectoryAllowed(file.GetFileMeta().GetName()) - if !allowed { - return fmt.Errorf("file not in allowed directories %s", file.GetFileMeta().GetName()) + updatingFilesError = errors.Join(updatingFilesError, err) } } - return nil + return updatingFilesError } // DetermineFileActions compares two sets of files to determine the file action for each file. Returns a map of files @@ -838,37 +436,7 @@ func (fms *FileManagerService) UpdateManifestFile(currentFiles map[string]*mpi.F updatedFiles = manifestFiles } - return fms.writeManifestFile(updatedFiles) -} - -func (fms *FileManagerService) writeManifestFile(updatedFiles map[string]*model.ManifestFile) (writeError error) { - manifestJSON, err := json.MarshalIndent(updatedFiles, "", " ") - if err != nil { - return fmt.Errorf("unable to marshal manifest file json: %w", err) - } - - // 0755 allows read/execute for all, write for owner - if err = os.MkdirAll(fms.agentConfig.ManifestDir, dirPerm); err != nil { - return fmt.Errorf("unable to create directory %s: %w", fms.agentConfig.ManifestDir, err) - } - - // 0600 ensures only root can read/write - newFile, err := os.OpenFile(fms.manifestFilePath, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, filePerm) - if err != nil { - return fmt.Errorf("failed to read manifest file: %w", err) - } - defer func() { - if closeErr := newFile.Close(); closeErr != nil { - writeError = closeErr - } - }() - - _, err = newFile.Write(manifestJSON) - if err != nil { - return fmt.Errorf("failed to write manifest file: %w", err) - } - - return writeError + return fms.fileOperator.WriteManifestFile(updatedFiles, fms.agentConfig.ManifestDir, fms.manifestFilePath) } func (fms *FileManagerService) manifestFile() (map[string]*model.ManifestFile, map[string]*mpi.File, error) { diff --git a/internal/file/file_manager_service_test.go b/internal/file/file_manager_service_test.go index aa139810d..d46d66640 100644 --- a/internal/file/file_manager_service_test.go +++ b/internal/file/file_manager_service_test.go @@ -11,7 +11,6 @@ import ( "fmt" "os" "path/filepath" - "sync/atomic" "testing" "github.com/nginx/agent/v3/internal/model" @@ -28,146 +27,6 @@ import ( "github.com/stretchr/testify/require" ) -func TestFileManagerService_UpdateOverview(t *testing.T) { - ctx := context.Background() - - filePath := filepath.Join(t.TempDir(), "nginx.conf") - fileMeta := protos.FileMeta(filePath, "") - - fileContent := []byte("location /test {\n return 200 \"Test location\\n\";\n}") - fileHash := files.GenerateHash(fileContent) - - fileWriteErr := os.WriteFile(filePath, fileContent, 0o600) - require.NoError(t, fileWriteErr) - - overview := protos.FileOverview(filePath, fileHash) - - fakeFileServiceClient := &v1fakes.FakeFileServiceClient{} - fakeFileServiceClient.UpdateOverviewReturnsOnCall(0, &mpi.UpdateOverviewResponse{ - Overview: overview, - }, nil) - - fakeFileServiceClient.UpdateOverviewReturnsOnCall(1, &mpi.UpdateOverviewResponse{}, nil) - - fakeFileServiceClient.UpdateFileReturns(&mpi.UpdateFileResponse{}, nil) - - fileManagerService := NewFileManagerService(fakeFileServiceClient, types.AgentConfig()) - fileManagerService.SetIsConnected(true) - - err := fileManagerService.UpdateOverview(ctx, "123", []*mpi.File{ - { - FileMeta: fileMeta, - }, - }, 0) - - require.NoError(t, err) - assert.Equal(t, 2, fakeFileServiceClient.UpdateOverviewCallCount()) -} - -func TestFileManagerService_UpdateOverview_MaxIterations(t *testing.T) { - ctx := context.Background() - - filePath := filepath.Join(t.TempDir(), "nginx.conf") - fileMeta := protos.FileMeta(filePath, "") - - fileContent := []byte("location /test {\n return 200 \"Test location\\n\";\n}") - fileHash := files.GenerateHash(fileContent) - - fileWriteErr := os.WriteFile(filePath, fileContent, 0o600) - require.NoError(t, fileWriteErr) - - overview := protos.FileOverview(filePath, fileHash) - - fakeFileServiceClient := &v1fakes.FakeFileServiceClient{} - - // do 5 iterations - for i := 0; i <= 5; i++ { - fakeFileServiceClient.UpdateOverviewReturnsOnCall(i, &mpi.UpdateOverviewResponse{ - Overview: overview, - }, nil) - } - - fakeFileServiceClient.UpdateFileReturns(&mpi.UpdateFileResponse{}, nil) - - fileManagerService := NewFileManagerService(fakeFileServiceClient, types.AgentConfig()) - fileManagerService.SetIsConnected(true) - - err := fileManagerService.UpdateOverview(ctx, "123", []*mpi.File{ - { - FileMeta: fileMeta, - }, - }, 0) - - require.Error(t, err) - assert.Equal(t, "too many UpdateOverview attempts", err.Error()) -} - -func TestFileManagerService_UpdateFile(t *testing.T) { - tests := []struct { - name string - isCert bool - }{ - { - name: "non-cert", - isCert: false, - }, - { - name: "cert", - isCert: true, - }, - } - - tempDir := os.TempDir() - - for _, test := range tests { - ctx := context.Background() - - testFile := helpers.CreateFileWithErrorCheck(t, tempDir, "nginx.conf") - - var fileMeta *mpi.FileMeta - if test.isCert { - fileMeta = protos.CertMeta(testFile.Name(), "") - } else { - fileMeta = protos.FileMeta(testFile.Name(), "") - } - - fakeFileServiceClient := &v1fakes.FakeFileServiceClient{} - fileManagerService := NewFileManagerService(fakeFileServiceClient, types.AgentConfig()) - fileManagerService.SetIsConnected(true) - - err := fileManagerService.UpdateFile(ctx, "123", &mpi.File{FileMeta: fileMeta}) - - require.NoError(t, err) - assert.Equal(t, 1, fakeFileServiceClient.UpdateFileCallCount()) - - helpers.RemoveFileWithErrorCheck(t, testFile.Name()) - } -} - -func TestFileManagerService_UpdateFile_LargeFile(t *testing.T) { - ctx := context.Background() - tempDir := os.TempDir() - - testFile := helpers.CreateFileWithErrorCheck(t, tempDir, "nginx.conf") - writeFileError := os.WriteFile(testFile.Name(), []byte("#test content"), 0o600) - require.NoError(t, writeFileError) - fileMeta := protos.FileMetaLargeFile(testFile.Name(), "") - - fakeFileServiceClient := &v1fakes.FakeFileServiceClient{} - fakeClientStreamingClient := &FakeClientStreamingClient{sendCount: atomic.Int32{}} - fakeFileServiceClient.UpdateFileStreamReturns(fakeClientStreamingClient, nil) - fileManagerService := NewFileManagerService(fakeFileServiceClient, types.AgentConfig()) - fileManagerService.SetIsConnected(true) - - err := fileManagerService.UpdateFile(ctx, "123", &mpi.File{FileMeta: fileMeta}) - - require.NoError(t, err) - assert.Equal(t, 0, fakeFileServiceClient.UpdateFileCallCount()) - assert.Equal(t, 14, int(fakeClientStreamingClient.sendCount.Load())) - - helpers.RemoveFileWithErrorCheck(t, testFile.Name()) -} - func TestFileManagerService_ConfigApply_Add(t *testing.T) { ctx := context.Background() tempDir := t.TempDir() diff --git a/internal/file/file_operator.go b/internal/file/file_operator.go index 456f1127b..88539ce3a 100644 --- a/internal/file/file_operator.go +++ b/internal/file/file_operator.go @@ -8,12 +8,15 @@ package file import ( "bufio" "context" + "encoding/json" "fmt" "io" "log/slog" "os" "path" + "github.com/nginx/agent/v3/internal/model" + "google.golang.org/grpc" "github.com/nginx/agent/v3/pkg/files" @@ -140,3 +143,35 @@ func (fo *FileOperator) ReadChunk( return chunk, err } + +func (fo *FileOperator) WriteManifestFile(updatedFiles map[string]*model.ManifestFile, manifestDir, + manifestPath string, +) (writeError error) { + manifestJSON, err := json.MarshalIndent(updatedFiles, "", " ") + if err != nil { + return fmt.Errorf("unable to marshal manifest file json: %w", err) + } + + // 0755 allows read/execute for all, write for owner + if err = os.MkdirAll(manifestDir, dirPerm); err != nil { + return fmt.Errorf("unable to create directory %s: %w", manifestDir, err) + } + + // 0600 ensures only root can read/write + newFile, err := os.OpenFile(manifestPath, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, filePerm) + if err != nil { + return fmt.Errorf("failed to read manifest file: %w", err) + } + defer func() { + if closeErr := newFile.Close(); closeErr != nil { + writeError = closeErr + } + }() + + _, err = newFile.Write(manifestJSON) + if err != nil { + return fmt.Errorf("failed to write manifest file: %w", err) + } + + return writeError +} diff --git a/internal/file/file_plugin.go b/internal/file/file_plugin.go index e402fae27..67e2138ed 100644 --- a/internal/file/file_plugin.go +++ b/internal/file/file_plugin.go @@ -7,7 +7,6 @@ package file import ( "context" - "fmt" "log/slog" "github.com/nginx/agent/v3/pkg/files" @@ -319,27 +318,10 @@ func (fp *FilePlugin) handleNginxConfigUpdate(ctx context.Context, msg *bus.Mess return } - updateError := fp.fileManagerService.UpdateCurrentFilesOnDisk( - ctx, - files.ConvertToMapOfFiles(nginxConfigContext.Files), - true, - ) - if updateError != nil { - slog.ErrorContext(ctx, "Unable to update current files on disk", "error", updateError) - } - - slog.InfoContext(ctx, "Updating overview after nginx config update") - err := fp.fileManagerService.UpdateOverview(ctx, nginxConfigContext.InstanceID, nginxConfigContext.Files, 0) - if err != nil { - slog.ErrorContext( - ctx, - "Failed to update file overview", - "instance_id", nginxConfigContext.InstanceID, - "error", err, - ) - } + fp.fileManagerService.ConfigUpdate(ctx, nginxConfigContext) } +// nolint: dupl func (fp *FilePlugin) handleConfigUploadRequest(ctx context.Context, msg *bus.Message) { slog.DebugContext(ctx, "File plugin received config upload request message") managementPlaneRequest, ok := msg.Data.(*mpi.ManagementPlaneRequest) @@ -357,36 +339,7 @@ func (fp *FilePlugin) handleConfigUploadRequest(ctx context.Context, msg *bus.Me correlationID := logger.CorrelationID(ctx) - var updatingFilesError error - - for _, file := range configUploadRequest.GetOverview().GetFiles() { - err := fp.fileManagerService.UpdateFile( - ctx, - configUploadRequest.GetOverview().GetConfigVersion().GetInstanceId(), - file, - ) - if err != nil { - slog.ErrorContext( - ctx, - "Failed to update file", - "instance_id", configUploadRequest.GetOverview().GetConfigVersion().GetInstanceId(), - "file_name", file.GetFileMeta().GetName(), - "error", err, - ) - - response := fp.createDataPlaneResponse(correlationID, mpi.CommandResponse_COMMAND_STATUS_ERROR, - fmt.Sprintf("Failed to update file %s", file.GetFileMeta().GetName()), - configUploadRequest.GetOverview().GetConfigVersion().GetInstanceId(), - err.Error(), - ) - - updatingFilesError = err - - fp.messagePipe.Process(ctx, &bus.Message{Topic: bus.DataPlaneResponseTopic, Data: response}) - - break - } - } + updatingFilesError := fp.fileManagerService.ConfigUpload(ctx, configUploadRequest) response := &mpi.DataPlaneResponse{ MessageMeta: &mpi.MessageMeta{ diff --git a/internal/file/file_plugin_test.go b/internal/file/file_plugin_test.go index fb7e556f5..c008d9de6 100644 --- a/internal/file/file_plugin_test.go +++ b/internal/file/file_plugin_test.go @@ -63,6 +63,7 @@ func TestFilePlugin_Subscriptions(t *testing.T) { ) } +// nolint: dupl func TestFilePlugin_Process_NginxConfigUpdateTopic(t *testing.T) { ctx := context.Background() @@ -224,6 +225,7 @@ func TestFilePlugin_Process_ConfigApplyRequestTopic(t *testing.T) { } } +// nolint: dupl func TestFilePlugin_Process_ConfigUploadRequestTopic(t *testing.T) { ctx := context.Background() @@ -322,7 +324,7 @@ func TestFilePlugin_Process_ConfigUploadRequestTopic_Failure(t *testing.T) { assert.Eventually( t, - func() bool { return len(messagePipe.Messages()) == 2 }, + func() bool { return len(messagePipe.Messages()) == 1 }, 2*time.Second, 10*time.Millisecond, ) @@ -330,21 +332,12 @@ func TestFilePlugin_Process_ConfigUploadRequestTopic_Failure(t *testing.T) { assert.Equal(t, 0, fakeFileServiceClient.UpdateFileCallCount()) messages := messagePipe.Messages() - assert.Len(t, messages, 2) + assert.Len(t, messages, 1) + assert.Equal(t, bus.DataPlaneResponseTopic, messages[0].Topic) dataPlaneResponse, ok := messages[0].Data.(*mpi.DataPlaneResponse) assert.True(t, ok) - assert.Equal( - t, - mpi.CommandResponse_COMMAND_STATUS_ERROR, - dataPlaneResponse.GetCommandResponse().GetStatus(), - ) - - assert.Equal(t, bus.DataPlaneResponseTopic, messages[1].Topic) - - dataPlaneResponse, ok = messages[1].Data.(*mpi.DataPlaneResponse) - assert.True(t, ok) assert.Equal( t, mpi.CommandResponse_COMMAND_STATUS_FAILURE, diff --git a/internal/file/file_service_operator.go b/internal/file/file_service_operator.go new file mode 100644 index 000000000..a3091c0c2 --- /dev/null +++ b/internal/file/file_service_operator.go @@ -0,0 +1,512 @@ +// Copyright (c) F5, Inc. +// +// This source code is licensed under the Apache License, Version 2.0 license found in the +// LICENSE file in the root directory of this source tree. + +package file + +import ( + "bufio" + "context" + "errors" + "fmt" + "log/slog" + "maps" + "math" + "os" + "slices" + "sync/atomic" + + "github.com/cenkalti/backoff/v4" + mpi "github.com/nginx/agent/v3/api/grpc/mpi/v1" + backoffHelpers "github.com/nginx/agent/v3/internal/backoff" + "github.com/nginx/agent/v3/internal/config" + internalgrpc "github.com/nginx/agent/v3/internal/grpc" + "github.com/nginx/agent/v3/internal/logger" + "github.com/nginx/agent/v3/internal/model" + "github.com/nginx/agent/v3/pkg/files" + "github.com/nginx/agent/v3/pkg/id" + "google.golang.org/grpc" + "google.golang.org/protobuf/types/known/timestamppb" +) + +type FileServiceOperator struct { + fileServiceClient mpi.FileServiceClient + agentConfig *config.Config + fileOperator fileOperator + isConnected *atomic.Bool +} + +var _ fileServiceOperatorInterface = (*FileServiceOperator)(nil) + +func NewFileServiceOperator(agentConfig *config.Config, fileServiceClient mpi.FileServiceClient) *FileServiceOperator { + isConnected := &atomic.Bool{} + isConnected.Store(false) + + return &FileServiceOperator{ + fileServiceClient: fileServiceClient, + agentConfig: agentConfig, + fileOperator: NewFileOperator(), + isConnected: isConnected, + } +} + +func (fso *FileServiceOperator) SetIsConnected(isConnected bool) { + fso.isConnected.Store(isConnected) +} + +func (fso *FileServiceOperator) IsConnected() bool { + return fso.isConnected.Load() +} + +func (fso *FileServiceOperator) File(ctx context.Context, file *mpi.File, + fileActions map[string]*model.FileCache, +) error { + slog.DebugContext(ctx, "Getting file", "file", file.GetFileMeta().GetName()) + + backOffCtx, backoffCancel := context.WithTimeout(ctx, fso.agentConfig.Client.Backoff.MaxElapsedTime) + defer backoffCancel() + + getFile := func() (*mpi.GetFileResponse, error) { + return fso.fileServiceClient.GetFile(ctx, &mpi.GetFileRequest{ + MessageMeta: &mpi.MessageMeta{ + MessageId: id.GenerateMessageID(), + CorrelationId: logger.CorrelationID(ctx), + Timestamp: timestamppb.Now(), + }, + FileMeta: file.GetFileMeta(), + }) + } + + getFileResp, getFileErr := backoff.RetryWithData( + getFile, + backoffHelpers.Context(backOffCtx, fso.agentConfig.Client.Backoff), + ) + + if getFileErr != nil { + return fmt.Errorf("error getting file data for %s: %w", file.GetFileMeta(), getFileErr) + } + + if writeErr := fso.fileOperator.Write(ctx, getFileResp.GetContents().GetContents(), + file.GetFileMeta()); writeErr != nil { + return writeErr + } + + return fso.validateFileHash(file.GetFileMeta().GetName(), fileActions) +} + +func (fso *FileServiceOperator) validateFileHash(filePath string, fileActions map[string]*model.FileCache) error { + content, err := os.ReadFile(filePath) + if err != nil { + return err + } + fileHash := files.GenerateHash(content) + + if fileHash != fileActions[filePath].File.GetFileMeta().GetHash() { + return fmt.Errorf("error writing file, file hash does not match for file %s", filePath) + } + + return nil +} + +func (fso *FileServiceOperator) UpdateOverview( + ctx context.Context, + instanceID string, + filesToUpdate []*mpi.File, + iteration int, +) error { + correlationID := logger.CorrelationID(ctx) + + // error case for the UpdateOverview attempts + if iteration > maxAttempts { + return errors.New("too many UpdateOverview attempts") + } + + newCtx, correlationID := fso.setupIdentifiers(ctx, iteration) + + request := &mpi.UpdateOverviewRequest{ + MessageMeta: &mpi.MessageMeta{ + MessageId: id.GenerateMessageID(), + CorrelationId: correlationID, + Timestamp: timestamppb.Now(), + }, + Overview: &mpi.FileOverview{ + Files: filesToUpdate, + ConfigVersion: &mpi.ConfigVersion{ + InstanceId: instanceID, + Version: files.GenerateConfigVersion(filesToUpdate), + }, + }, + } + + backOffCtx, backoffCancel := context.WithTimeout(newCtx, fso.agentConfig.Client.Backoff.MaxElapsedTime) + defer backoffCancel() + + sendUpdateOverview := func() (*mpi.UpdateOverviewResponse, error) { + if fso.fileServiceClient == nil { + return nil, errors.New("file service client is not initialized") + } + + if !fso.isConnected.Load() { + return nil, errors.New("CreateConnection rpc has not being called yet") + } + + slog.DebugContext(newCtx, "Sending update overview request", + "instance_id", request.GetOverview().GetConfigVersion().GetInstanceId(), + "request", request, "parent_correlation_id", correlationID, + ) + + response, updateError := fso.fileServiceClient.UpdateOverview(newCtx, request) + + validatedError := internalgrpc.ValidateGrpcError(updateError) + + if validatedError != nil { + slog.ErrorContext(newCtx, "Failed to send update overview", "error", validatedError) + + return nil, validatedError + } + + return response, nil + } + + backoffSettings := fso.agentConfig.Client.Backoff + response, err := backoff.RetryWithData( + sendUpdateOverview, + backoffHelpers.Context(backOffCtx, backoffSettings), + ) + if err != nil { + return err + } + + slog.DebugContext(newCtx, "UpdateOverview response", "response", response) + + if response.GetOverview() == nil { + slog.Debug("UpdateOverview response is empty") + return nil + } + delta := files.ConvertToMapOfFiles(response.GetOverview().GetFiles()) + + if len(delta) != 0 { + return fso.updateFiles(ctx, delta, instanceID, iteration) + } + + return err +} + +func (fso *FileServiceOperator) updateFiles( + ctx context.Context, + delta map[string]*mpi.File, + instanceID string, + iteration int, +) error { + diffFiles := slices.Collect(maps.Values(delta)) + + for _, file := range diffFiles { + updateErr := fso.UpdateFile(ctx, instanceID, file) + if updateErr != nil { + return updateErr + } + } + + iteration++ + slog.Info("Updating file overview after file updates", "attempt_number", iteration) + + return fso.UpdateOverview(ctx, instanceID, diffFiles, iteration) +} + +func (fso *FileServiceOperator) UpdateFile( + ctx context.Context, + instanceID string, + fileToUpdate *mpi.File, +) error { + slog.InfoContext(ctx, "Updating file", "file_name", fileToUpdate.GetFileMeta().GetName(), "instance_id", instanceID) + + slog.DebugContext(ctx, "Checking file size", + "file_size", fileToUpdate.GetFileMeta().GetSize(), + "max_file_size", int64(fso.agentConfig.Client.Grpc.MaxFileSize), + ) + + if fileToUpdate.GetFileMeta().GetSize() <= int64(fso.agentConfig.Client.Grpc.MaxFileSize) { + return fso.sendUpdateFileRequest(ctx, fileToUpdate) + } + + return fso.sendUpdateFileStream(ctx, fileToUpdate, fso.agentConfig.Client.Grpc.FileChunkSize) +} + +func (fso *FileServiceOperator) sendUpdateFileRequest( + ctx context.Context, + fileToUpdate *mpi.File, +) error { + messageMeta := &mpi.MessageMeta{ + MessageId: id.GenerateMessageID(), + CorrelationId: logger.CorrelationID(ctx), + Timestamp: timestamppb.Now(), + } + + contents, err := os.ReadFile(fileToUpdate.GetFileMeta().GetName()) + if err != nil { + return err + } + + request := &mpi.UpdateFileRequest{ + File: fileToUpdate, + Contents: &mpi.FileContents{ + Contents: contents, + }, + MessageMeta: messageMeta, + } + + backOffCtx, backoffCancel := context.WithTimeout(ctx, fso.agentConfig.Client.Backoff.MaxElapsedTime) + defer backoffCancel() + + sendUpdateFile := func() (*mpi.UpdateFileResponse, error) { + slog.DebugContext(ctx, "Sending update file request", "request_file", request.GetFile(), + "request_message_meta", request.GetMessageMeta()) + if fso.fileServiceClient == nil { + return nil, errors.New("file service client is not initialized") + } + + if !fso.isConnected.Load() { + return nil, errors.New("CreateConnection rpc has not being called yet") + } + + response, updateError := fso.fileServiceClient.UpdateFile(ctx, request) + + validatedError := internalgrpc.ValidateGrpcError(updateError) + + if validatedError != nil { + slog.ErrorContext(ctx, "Failed to send update file", "error", validatedError) + + return nil, validatedError + } + + return response, nil + } + + response, err := backoff.RetryWithData( + sendUpdateFile, + backoffHelpers.Context(backOffCtx, fso.agentConfig.Client.Backoff), + ) + if err != nil { + return err + } + + slog.DebugContext(ctx, "UpdateFile response", "response", response) + + return nil +} + +func (fso *FileServiceOperator) sendUpdateFileStream( + ctx context.Context, + fileToUpdate *mpi.File, + chunkSize uint32, +) error { + if chunkSize == 0 { + return fmt.Errorf("file chunk size must be greater than zero") + } + + updateFileStreamClient, err := fso.fileServiceClient.UpdateFileStream(ctx) + if err != nil { + return err + } + + err = fso.sendUpdateFileStreamHeader(ctx, fileToUpdate, chunkSize, updateFileStreamClient) + if err != nil { + return err + } + + return fso.sendFileUpdateStreamChunks(ctx, fileToUpdate, chunkSize, updateFileStreamClient) +} + +func (fso *FileServiceOperator) sendUpdateFileStreamHeader( + ctx context.Context, + fileToUpdate *mpi.File, + chunkSize uint32, + updateFileStreamClient grpc.ClientStreamingClient[mpi.FileDataChunk, mpi.UpdateFileResponse], +) error { + messageMeta := &mpi.MessageMeta{ + MessageId: id.GenerateMessageID(), + CorrelationId: logger.CorrelationID(ctx), + Timestamp: timestamppb.Now(), + } + + numberOfChunks := uint32(math.Ceil(float64(fileToUpdate.GetFileMeta().GetSize()) / float64(chunkSize))) + + header := mpi.FileDataChunk_Header{ + Header: &mpi.FileDataChunkHeader{ + FileMeta: fileToUpdate.GetFileMeta(), + Chunks: numberOfChunks, + ChunkSize: chunkSize, + }, + } + + backOffCtx, backoffCancel := context.WithTimeout(ctx, fso.agentConfig.Client.Backoff.MaxElapsedTime) + defer backoffCancel() + + sendUpdateFileHeader := func() error { + slog.DebugContext(ctx, "Sending update file stream header", "header", header) + if fso.fileServiceClient == nil { + return errors.New("file service client is not initialized") + } + + if !fso.isConnected.Load() { + return errors.New("CreateConnection rpc has not being called yet") + } + + err := updateFileStreamClient.Send( + &mpi.FileDataChunk{ + Meta: messageMeta, + Chunk: &header, + }, + ) + + validatedError := internalgrpc.ValidateGrpcError(err) + + if validatedError != nil { + slog.ErrorContext(ctx, "Failed to send update file stream header", "error", validatedError) + + return validatedError + } + + return nil + } + + return backoff.Retry(sendUpdateFileHeader, backoffHelpers.Context(backOffCtx, fso.agentConfig.Client.Backoff)) +} + +func (fso *FileServiceOperator) sendFileUpdateStreamChunks( + ctx context.Context, + fileToUpdate *mpi.File, + chunkSize uint32, + updateFileStreamClient grpc.ClientStreamingClient[mpi.FileDataChunk, mpi.UpdateFileResponse], +) error { + f, err := os.Open(fileToUpdate.GetFileMeta().GetName()) + defer func() { + closeError := f.Close() + if closeError != nil { + slog.WarnContext( + ctx, "Failed to close file", + "file", fileToUpdate.GetFileMeta().GetName(), + "error", closeError, + ) + } + }() + if err != nil { + return err + } + + var chunkID uint32 + + reader := bufio.NewReader(f) + for { + chunk, readChunkError := fso.fileOperator.ReadChunk(ctx, chunkSize, reader, chunkID) + if readChunkError != nil { + return readChunkError + } + if chunk.Content == nil { + break + } + + sendError := fso.sendFileUpdateStreamChunk(ctx, chunk, updateFileStreamClient) + if sendError != nil { + return sendError + } + + chunkID++ + } + + return nil +} + +func (fso *FileServiceOperator) sendFileUpdateStreamChunk( + ctx context.Context, + chunk mpi.FileDataChunk_Content, + updateFileStreamClient grpc.ClientStreamingClient[mpi.FileDataChunk, mpi.UpdateFileResponse], +) error { + messageMeta := &mpi.MessageMeta{ + MessageId: id.GenerateMessageID(), + CorrelationId: logger.CorrelationID(ctx), + Timestamp: timestamppb.Now(), + } + + backOffCtx, backoffCancel := context.WithTimeout(ctx, fso.agentConfig.Client.Backoff.MaxElapsedTime) + defer backoffCancel() + + sendUpdateFileChunk := func() error { + slog.DebugContext(ctx, "Sending update file stream chunk", "chunk_id", chunk.Content.GetChunkId()) + if fso.fileServiceClient == nil { + return errors.New("file service client is not initialized") + } + + if !fso.isConnected.Load() { + return errors.New("CreateConnection rpc has not being called yet") + } + + err := updateFileStreamClient.Send( + &mpi.FileDataChunk{ + Meta: messageMeta, + Chunk: &chunk, + }, + ) + + validatedError := internalgrpc.ValidateGrpcError(err) + + if validatedError != nil { + slog.ErrorContext(ctx, "Failed to send update file stream chunk", "error", validatedError) + + return validatedError + } + + return nil + } + + return backoff.Retry(sendUpdateFileChunk, backoffHelpers.Context(backOffCtx, fso.agentConfig.Client.Backoff)) +} + +func (fso *FileServiceOperator) ChunkedFile(ctx context.Context, file *mpi.File) error { + slog.DebugContext(ctx, "Getting chunked file", "file", file.GetFileMeta().GetName()) + + stream, err := fso.fileServiceClient.GetFileStream(ctx, &mpi.GetFileRequest{ + MessageMeta: &mpi.MessageMeta{ + MessageId: id.GenerateMessageID(), + CorrelationId: logger.CorrelationID(ctx), + Timestamp: timestamppb.Now(), + }, + FileMeta: file.GetFileMeta(), + }) + if err != nil { + return fmt.Errorf("error getting file stream for %s: %w", file.GetFileMeta().GetName(), err) + } + + // Get header chunk first + headerChunk, recvHeaderChunkError := stream.Recv() + if recvHeaderChunkError != nil { + return recvHeaderChunkError + } + + slog.DebugContext(ctx, "File header chunk received", "header_chunk", headerChunk) + + header := headerChunk.GetHeader() + + writeChunkedFileError := fso.fileOperator.WriteChunkedFile(ctx, file, header, stream) + if writeChunkedFileError != nil { + return writeChunkedFileError + } + + return nil +} + +func (fso *FileServiceOperator) setupIdentifiers(ctx context.Context, iteration int) (context.Context, string) { + correlationID := logger.CorrelationID(ctx) + var requestCorrelationID slog.Attr + + if iteration == 0 { + requestCorrelationID = logger.GenerateCorrelationID() + } else { + requestCorrelationID = logger.CorrelationIDAttr(ctx) + } + + newCtx := context.WithValue(ctx, logger.CorrelationIDContextKey, requestCorrelationID) + + return newCtx, correlationID +} diff --git a/internal/file/file_service_operator_test.go b/internal/file/file_service_operator_test.go new file mode 100644 index 000000000..632749fbb --- /dev/null +++ b/internal/file/file_service_operator_test.go @@ -0,0 +1,163 @@ +// Copyright (c) F5, Inc. +// +// This source code is licensed under the Apache License, Version 2.0 license found in the +// LICENSE file in the root directory of this source tree. + +package file + +import ( + "context" + "os" + "path/filepath" + "sync/atomic" + "testing" + + mpi "github.com/nginx/agent/v3/api/grpc/mpi/v1" + "github.com/nginx/agent/v3/api/grpc/mpi/v1/v1fakes" + "github.com/nginx/agent/v3/pkg/files" + "github.com/nginx/agent/v3/test/helpers" + "github.com/nginx/agent/v3/test/protos" + "github.com/nginx/agent/v3/test/types" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestFileServiceOperator_UpdateOverview(t *testing.T) { + ctx := context.Background() + + filePath := filepath.Join(t.TempDir(), "nginx.conf") + fileMeta := protos.FileMeta(filePath, "") + + fileContent := []byte("location /test {\n return 200 \"Test location\\n\";\n}") + fileHash := files.GenerateHash(fileContent) + + fileWriteErr := os.WriteFile(filePath, fileContent, 0o600) + require.NoError(t, fileWriteErr) + + overview := protos.FileOverview(filePath, fileHash) + + fakeFileServiceClient := &v1fakes.FakeFileServiceClient{} + fakeFileServiceClient.UpdateOverviewReturnsOnCall(0, &mpi.UpdateOverviewResponse{ + Overview: overview, + }, nil) + + fakeFileServiceClient.UpdateOverviewReturnsOnCall(1, &mpi.UpdateOverviewResponse{}, nil) + + fakeFileServiceClient.UpdateFileReturns(&mpi.UpdateFileResponse{}, nil) + + fileServiceOperator := NewFileServiceOperator(types.AgentConfig(), fakeFileServiceClient) + fileServiceOperator.SetIsConnected(true) + + err := fileServiceOperator.UpdateOverview(ctx, "123", []*mpi.File{ + { + FileMeta: fileMeta, + }, + }, 0) + + require.NoError(t, err) + assert.Equal(t, 2, fakeFileServiceClient.UpdateOverviewCallCount()) +} + +func TestFileServiceOperator_UpdateOverview_MaxIterations(t *testing.T) { + ctx := context.Background() + + filePath := filepath.Join(t.TempDir(), "nginx.conf") + fileMeta := protos.FileMeta(filePath, "") + + fileContent := []byte("location /test {\n return 200 \"Test location\\n\";\n}") + fileHash := files.GenerateHash(fileContent) + + fileWriteErr := os.WriteFile(filePath, fileContent, 0o600) + require.NoError(t, fileWriteErr) + + overview := protos.FileOverview(filePath, fileHash) + + fakeFileServiceClient := &v1fakes.FakeFileServiceClient{} + + // do 5 iterations + for i := 0; i <= 5; i++ { + fakeFileServiceClient.UpdateOverviewReturnsOnCall(i, &mpi.UpdateOverviewResponse{ + Overview: overview, + }, nil) + } + + fakeFileServiceClient.UpdateFileReturns(&mpi.UpdateFileResponse{}, nil) + + fileServiceOperator := NewFileServiceOperator(types.AgentConfig(), fakeFileServiceClient) + fileServiceOperator.SetIsConnected(true) + + err := fileServiceOperator.UpdateOverview(ctx, "123", []*mpi.File{ + { + FileMeta: fileMeta, + }, + }, 0) + + require.Error(t, err) + assert.Equal(t, "too many UpdateOverview attempts", err.Error()) +} + +func TestFileManagerService_UpdateFile(t *testing.T) { + tests := []struct { + name string + isCert bool + }{ + { + name: "non-cert", + isCert: false, + }, + { + name: "cert", + isCert: true, + }, + } + + tempDir := os.TempDir() + + for _, test := range tests { + ctx := context.Background() + + testFile := helpers.CreateFileWithErrorCheck(t, tempDir, "nginx.conf") + + var fileMeta *mpi.FileMeta + if test.isCert { + fileMeta = protos.CertMeta(testFile.Name(), "") + } else { + fileMeta = protos.FileMeta(testFile.Name(), "") + } + + fakeFileServiceClient := &v1fakes.FakeFileServiceClient{} + fileServiceOperator := NewFileServiceOperator(types.AgentConfig(), fakeFileServiceClient) + fileServiceOperator.SetIsConnected(true) + + err := fileServiceOperator.UpdateFile(ctx, "123", &mpi.File{FileMeta: fileMeta}) + + require.NoError(t, err) + assert.Equal(t, 1, fakeFileServiceClient.UpdateFileCallCount()) + + helpers.RemoveFileWithErrorCheck(t, testFile.Name()) + } +} + +func TestFileManagerService_UpdateFile_LargeFile(t *testing.T) { + ctx := context.Background() + tempDir := os.TempDir() + + testFile := helpers.CreateFileWithErrorCheck(t, tempDir, "nginx.conf") + writeFileError := os.WriteFile(testFile.Name(), []byte("#test content"), 0o600) + require.NoError(t, writeFileError) + fileMeta := protos.FileMetaLargeFile(testFile.Name(), "") + + fakeFileServiceClient := &v1fakes.FakeFileServiceClient{} + fakeClientStreamingClient := &FakeClientStreamingClient{sendCount: atomic.Int32{}} + fakeFileServiceClient.UpdateFileStreamReturns(fakeClientStreamingClient, nil) + fileServiceOperator := NewFileServiceOperator(types.AgentConfig(), fakeFileServiceClient) + + fileServiceOperator.SetIsConnected(true) + err := fileServiceOperator.UpdateFile(ctx, "123", &mpi.File{FileMeta: fileMeta}) + + require.NoError(t, err) + assert.Equal(t, 0, fakeFileServiceClient.UpdateFileCallCount()) + assert.Equal(t, 14, int(fakeClientStreamingClient.sendCount.Load())) + + helpers.RemoveFileWithErrorCheck(t, testFile.Name()) +} diff --git a/internal/file/filefakes/fake_file_manager_service_interface.go b/internal/file/filefakes/fake_file_manager_service_interface.go index 6a494e5b6..bf43e0855 100644 --- a/internal/file/filefakes/fake_file_manager_service_interface.go +++ b/internal/file/filefakes/fake_file_manager_service_interface.go @@ -28,6 +28,24 @@ type FakeFileManagerServiceInterface struct { result1 model.WriteStatus result2 error } + ConfigUpdateStub func(context.Context, *model.NginxConfigContext) + configUpdateMutex sync.RWMutex + configUpdateArgsForCall []struct { + arg1 context.Context + arg2 *model.NginxConfigContext + } + ConfigUploadStub func(context.Context, *v1.ConfigUploadRequest) error + configUploadMutex sync.RWMutex + configUploadArgsForCall []struct { + arg1 context.Context + arg2 *v1.ConfigUploadRequest + } + configUploadReturns struct { + result1 error + } + configUploadReturnsOnCall map[int]struct { + result1 error + } DetermineFileActionsStub func(map[string]*v1.File, map[string]*model.FileCache) (map[string]*model.FileCache, map[string][]byte, error) determineFileActionsMutex sync.RWMutex determineFileActionsArgsForCall []struct { @@ -84,33 +102,6 @@ type FakeFileManagerServiceInterface struct { updateCurrentFilesOnDiskReturnsOnCall map[int]struct { result1 error } - UpdateFileStub func(context.Context, string, *v1.File) error - updateFileMutex sync.RWMutex - updateFileArgsForCall []struct { - arg1 context.Context - arg2 string - arg3 *v1.File - } - updateFileReturns struct { - result1 error - } - updateFileReturnsOnCall map[int]struct { - result1 error - } - UpdateOverviewStub func(context.Context, string, []*v1.File, int) error - updateOverviewMutex sync.RWMutex - updateOverviewArgsForCall []struct { - arg1 context.Context - arg2 string - arg3 []*v1.File - arg4 int - } - updateOverviewReturns struct { - result1 error - } - updateOverviewReturnsOnCall map[int]struct { - result1 error - } invocations map[string][][]interface{} invocationsMutex sync.RWMutex } @@ -204,6 +195,101 @@ func (fake *FakeFileManagerServiceInterface) ConfigApplyReturnsOnCall(i int, res }{result1, result2} } +func (fake *FakeFileManagerServiceInterface) ConfigUpdate(arg1 context.Context, arg2 *model.NginxConfigContext) { + fake.configUpdateMutex.Lock() + fake.configUpdateArgsForCall = append(fake.configUpdateArgsForCall, struct { + arg1 context.Context + arg2 *model.NginxConfigContext + }{arg1, arg2}) + stub := fake.ConfigUpdateStub + fake.recordInvocation("ConfigUpdate", []interface{}{arg1, arg2}) + fake.configUpdateMutex.Unlock() + if stub != nil { + fake.ConfigUpdateStub(arg1, arg2) + } +} + +func (fake *FakeFileManagerServiceInterface) ConfigUpdateCallCount() int { + fake.configUpdateMutex.RLock() + defer fake.configUpdateMutex.RUnlock() + return len(fake.configUpdateArgsForCall) +} + +func (fake *FakeFileManagerServiceInterface) ConfigUpdateCalls(stub func(context.Context, *model.NginxConfigContext)) { + fake.configUpdateMutex.Lock() + defer fake.configUpdateMutex.Unlock() + fake.ConfigUpdateStub = stub +} + +func (fake *FakeFileManagerServiceInterface) ConfigUpdateArgsForCall(i int) (context.Context, *model.NginxConfigContext) { + fake.configUpdateMutex.RLock() + defer fake.configUpdateMutex.RUnlock() + argsForCall := fake.configUpdateArgsForCall[i] + return argsForCall.arg1, argsForCall.arg2 +} + +func (fake *FakeFileManagerServiceInterface) ConfigUpload(arg1 context.Context, arg2 *v1.ConfigUploadRequest) error { + fake.configUploadMutex.Lock() + ret, specificReturn := fake.configUploadReturnsOnCall[len(fake.configUploadArgsForCall)] + fake.configUploadArgsForCall = append(fake.configUploadArgsForCall, struct { + arg1 context.Context + arg2 *v1.ConfigUploadRequest + }{arg1, arg2}) + stub := fake.ConfigUploadStub + fakeReturns := fake.configUploadReturns + fake.recordInvocation("ConfigUpload", []interface{}{arg1, arg2}) + fake.configUploadMutex.Unlock() + if stub != nil { + return stub(arg1, arg2) + } + if specificReturn { + return ret.result1 + } + return fakeReturns.result1 +} + +func (fake *FakeFileManagerServiceInterface) ConfigUploadCallCount() int { + fake.configUploadMutex.RLock() + defer fake.configUploadMutex.RUnlock() + return len(fake.configUploadArgsForCall) +} + +func (fake *FakeFileManagerServiceInterface) ConfigUploadCalls(stub func(context.Context, *v1.ConfigUploadRequest) error) { + fake.configUploadMutex.Lock() + defer fake.configUploadMutex.Unlock() + fake.ConfigUploadStub = stub +} + +func (fake *FakeFileManagerServiceInterface) ConfigUploadArgsForCall(i int) (context.Context, *v1.ConfigUploadRequest) { + fake.configUploadMutex.RLock() + defer fake.configUploadMutex.RUnlock() + argsForCall := fake.configUploadArgsForCall[i] + return argsForCall.arg1, argsForCall.arg2 +} + +func (fake *FakeFileManagerServiceInterface) ConfigUploadReturns(result1 error) { + fake.configUploadMutex.Lock() + defer fake.configUploadMutex.Unlock() + fake.ConfigUploadStub = nil + fake.configUploadReturns = struct { + result1 error + }{result1} +} + +func (fake *FakeFileManagerServiceInterface) ConfigUploadReturnsOnCall(i int, result1 error) { + fake.configUploadMutex.Lock() + defer fake.configUploadMutex.Unlock() + fake.ConfigUploadStub = nil + if fake.configUploadReturnsOnCall == nil { + fake.configUploadReturnsOnCall = make(map[int]struct { + result1 error + }) + } + fake.configUploadReturnsOnCall[i] = struct { + result1 error + }{result1} +} + func (fake *FakeFileManagerServiceInterface) DetermineFileActions(arg1 map[string]*v1.File, arg2 map[string]*model.FileCache) (map[string]*model.FileCache, map[string][]byte, error) { fake.determineFileActionsMutex.Lock() ret, specificReturn := fake.determineFileActionsReturnsOnCall[len(fake.determineFileActionsArgsForCall)] @@ -482,138 +568,6 @@ func (fake *FakeFileManagerServiceInterface) UpdateCurrentFilesOnDiskReturnsOnCa }{result1} } -func (fake *FakeFileManagerServiceInterface) UpdateFile(arg1 context.Context, arg2 string, arg3 *v1.File) error { - fake.updateFileMutex.Lock() - ret, specificReturn := fake.updateFileReturnsOnCall[len(fake.updateFileArgsForCall)] - fake.updateFileArgsForCall = append(fake.updateFileArgsForCall, struct { - arg1 context.Context - arg2 string - arg3 *v1.File - }{arg1, arg2, arg3}) - stub := fake.UpdateFileStub - fakeReturns := fake.updateFileReturns - fake.recordInvocation("UpdateFile", []interface{}{arg1, arg2, arg3}) - fake.updateFileMutex.Unlock() - if stub != nil { - return stub(arg1, arg2, arg3) - } - if specificReturn { - return ret.result1 - } - return fakeReturns.result1 -} - -func (fake *FakeFileManagerServiceInterface) UpdateFileCallCount() int { - fake.updateFileMutex.RLock() - defer fake.updateFileMutex.RUnlock() - return len(fake.updateFileArgsForCall) -} - -func (fake *FakeFileManagerServiceInterface) UpdateFileCalls(stub func(context.Context, string, *v1.File) error) { - fake.updateFileMutex.Lock() - defer fake.updateFileMutex.Unlock() - fake.UpdateFileStub = stub -} - -func (fake *FakeFileManagerServiceInterface) UpdateFileArgsForCall(i int) (context.Context, string, *v1.File) { - fake.updateFileMutex.RLock() - defer fake.updateFileMutex.RUnlock() - argsForCall := fake.updateFileArgsForCall[i] - return argsForCall.arg1, argsForCall.arg2, argsForCall.arg3 -} - -func (fake *FakeFileManagerServiceInterface) UpdateFileReturns(result1 error) { - fake.updateFileMutex.Lock() - defer fake.updateFileMutex.Unlock() - fake.UpdateFileStub = nil - fake.updateFileReturns = struct { - result1 error - }{result1} -} - -func (fake *FakeFileManagerServiceInterface) UpdateFileReturnsOnCall(i int, result1 error) { - fake.updateFileMutex.Lock() - defer fake.updateFileMutex.Unlock() - fake.UpdateFileStub = nil - if fake.updateFileReturnsOnCall == nil { - fake.updateFileReturnsOnCall = make(map[int]struct { - result1 error - }) - } - fake.updateFileReturnsOnCall[i] = struct { - result1 error - }{result1} -} - -func (fake *FakeFileManagerServiceInterface) UpdateOverview(arg1 context.Context, arg2 string, arg3 []*v1.File, arg4 int) error { - var arg3Copy []*v1.File - if arg3 != nil { - arg3Copy = make([]*v1.File, len(arg3)) - copy(arg3Copy, arg3) - } - fake.updateOverviewMutex.Lock() - ret, specificReturn := fake.updateOverviewReturnsOnCall[len(fake.updateOverviewArgsForCall)] - fake.updateOverviewArgsForCall = append(fake.updateOverviewArgsForCall, struct { - arg1 context.Context - arg2 string - arg3 []*v1.File - arg4 int - }{arg1, arg2, arg3Copy, arg4}) - stub := fake.UpdateOverviewStub - fakeReturns := fake.updateOverviewReturns - fake.recordInvocation("UpdateOverview", []interface{}{arg1, arg2, arg3Copy, arg4}) - fake.updateOverviewMutex.Unlock() - if stub != nil { - return stub(arg1, arg2, arg3, arg4) - } - if specificReturn { - return ret.result1 - } - return fakeReturns.result1 -} - -func (fake *FakeFileManagerServiceInterface) UpdateOverviewCallCount() int { - fake.updateOverviewMutex.RLock() - defer fake.updateOverviewMutex.RUnlock() - return len(fake.updateOverviewArgsForCall) -} - -func (fake *FakeFileManagerServiceInterface) UpdateOverviewCalls(stub func(context.Context, string, []*v1.File, int) error) { - fake.updateOverviewMutex.Lock() - defer fake.updateOverviewMutex.Unlock() - fake.UpdateOverviewStub = stub -} - -func (fake *FakeFileManagerServiceInterface) UpdateOverviewArgsForCall(i int) (context.Context, string, []*v1.File, int) { - fake.updateOverviewMutex.RLock() - defer fake.updateOverviewMutex.RUnlock() - argsForCall := fake.updateOverviewArgsForCall[i] - return argsForCall.arg1, argsForCall.arg2, argsForCall.arg3, argsForCall.arg4 -} - -func (fake *FakeFileManagerServiceInterface) UpdateOverviewReturns(result1 error) { - fake.updateOverviewMutex.Lock() - defer fake.updateOverviewMutex.Unlock() - fake.UpdateOverviewStub = nil - fake.updateOverviewReturns = struct { - result1 error - }{result1} -} - -func (fake *FakeFileManagerServiceInterface) UpdateOverviewReturnsOnCall(i int, result1 error) { - fake.updateOverviewMutex.Lock() - defer fake.updateOverviewMutex.Unlock() - fake.UpdateOverviewStub = nil - if fake.updateOverviewReturnsOnCall == nil { - fake.updateOverviewReturnsOnCall = make(map[int]struct { - result1 error - }) - } - fake.updateOverviewReturnsOnCall[i] = struct { - result1 error - }{result1} -} - func (fake *FakeFileManagerServiceInterface) Invocations() map[string][][]interface{} { fake.invocationsMutex.RLock() defer fake.invocationsMutex.RUnlock() @@ -621,6 +575,10 @@ func (fake *FakeFileManagerServiceInterface) Invocations() map[string][][]interf defer fake.clearCacheMutex.RUnlock() fake.configApplyMutex.RLock() defer fake.configApplyMutex.RUnlock() + fake.configUpdateMutex.RLock() + defer fake.configUpdateMutex.RUnlock() + fake.configUploadMutex.RLock() + defer fake.configUploadMutex.RUnlock() fake.determineFileActionsMutex.RLock() defer fake.determineFileActionsMutex.RUnlock() fake.isConnectedMutex.RLock() @@ -631,10 +589,6 @@ func (fake *FakeFileManagerServiceInterface) Invocations() map[string][][]interf defer fake.setIsConnectedMutex.RUnlock() fake.updateCurrentFilesOnDiskMutex.RLock() defer fake.updateCurrentFilesOnDiskMutex.RUnlock() - fake.updateFileMutex.RLock() - defer fake.updateFileMutex.RUnlock() - fake.updateOverviewMutex.RLock() - defer fake.updateOverviewMutex.RUnlock() copiedInvocations := map[string][][]interface{}{} for key, value := range fake.invocations { copiedInvocations[key] = value diff --git a/internal/file/filefakes/fake_file_operator.go b/internal/file/filefakes/fake_file_operator.go index 0f064b5a1..77445acdc 100644 --- a/internal/file/filefakes/fake_file_operator.go +++ b/internal/file/filefakes/fake_file_operator.go @@ -8,6 +8,7 @@ import ( "sync" v1 "github.com/nginx/agent/v3/api/grpc/mpi/v1" + "github.com/nginx/agent/v3/internal/model" "google.golang.org/grpc" ) @@ -68,6 +69,19 @@ type FakeFileOperator struct { writeChunkedFileReturnsOnCall map[int]struct { result1 error } + WriteManifestFileStub func(map[string]*model.ManifestFile, string, string) error + writeManifestFileMutex sync.RWMutex + writeManifestFileArgsForCall []struct { + arg1 map[string]*model.ManifestFile + arg2 string + arg3 string + } + writeManifestFileReturns struct { + result1 error + } + writeManifestFileReturnsOnCall map[int]struct { + result1 error + } invocations map[string][][]interface{} invocationsMutex sync.RWMutex } @@ -334,6 +348,69 @@ func (fake *FakeFileOperator) WriteChunkedFileReturnsOnCall(i int, result1 error }{result1} } +func (fake *FakeFileOperator) WriteManifestFile(arg1 map[string]*model.ManifestFile, arg2 string, arg3 string) error { + fake.writeManifestFileMutex.Lock() + ret, specificReturn := fake.writeManifestFileReturnsOnCall[len(fake.writeManifestFileArgsForCall)] + fake.writeManifestFileArgsForCall = append(fake.writeManifestFileArgsForCall, struct { + arg1 map[string]*model.ManifestFile + arg2 string + arg3 string + }{arg1, arg2, arg3}) + stub := fake.WriteManifestFileStub + fakeReturns := fake.writeManifestFileReturns + fake.recordInvocation("WriteManifestFile", []interface{}{arg1, arg2, arg3}) + fake.writeManifestFileMutex.Unlock() + if stub != nil { + return stub(arg1, arg2, arg3) + } + if specificReturn { + return ret.result1 + } + return fakeReturns.result1 +} + +func (fake *FakeFileOperator) WriteManifestFileCallCount() int { + fake.writeManifestFileMutex.RLock() + defer fake.writeManifestFileMutex.RUnlock() + return len(fake.writeManifestFileArgsForCall) +} + +func (fake *FakeFileOperator) WriteManifestFileCalls(stub func(map[string]*model.ManifestFile, string, string) error) { + fake.writeManifestFileMutex.Lock() + defer fake.writeManifestFileMutex.Unlock() + fake.WriteManifestFileStub = stub +} + +func (fake *FakeFileOperator) WriteManifestFileArgsForCall(i int) (map[string]*model.ManifestFile, string, string) { + fake.writeManifestFileMutex.RLock() + defer fake.writeManifestFileMutex.RUnlock() + argsForCall := fake.writeManifestFileArgsForCall[i] + return argsForCall.arg1, argsForCall.arg2, argsForCall.arg3 +} + +func (fake *FakeFileOperator) WriteManifestFileReturns(result1 error) { + fake.writeManifestFileMutex.Lock() + defer fake.writeManifestFileMutex.Unlock() + fake.WriteManifestFileStub = nil + fake.writeManifestFileReturns = struct { + result1 error + }{result1} +} + +func (fake *FakeFileOperator) WriteManifestFileReturnsOnCall(i int, result1 error) { + fake.writeManifestFileMutex.Lock() + defer fake.writeManifestFileMutex.Unlock() + fake.WriteManifestFileStub = nil + if fake.writeManifestFileReturnsOnCall == nil { + fake.writeManifestFileReturnsOnCall = make(map[int]struct { + result1 error + }) + } + fake.writeManifestFileReturnsOnCall[i] = struct { + result1 error + }{result1} +} + func (fake *FakeFileOperator) Invocations() map[string][][]interface{} { fake.invocationsMutex.RLock() defer fake.invocationsMutex.RUnlock() @@ -345,6 +422,8 @@ func (fake *FakeFileOperator) Invocations() map[string][][]interface{} { defer fake.writeMutex.RUnlock() fake.writeChunkedFileMutex.RLock() defer fake.writeChunkedFileMutex.RUnlock() + fake.writeManifestFileMutex.RLock() + defer fake.writeManifestFileMutex.RUnlock() copiedInvocations := map[string][][]interface{}{} for key, value := range fake.invocations { copiedInvocations[key] = value diff --git a/internal/file/read_only_plugin.go b/internal/file/read_only_plugin.go new file mode 100644 index 000000000..fda214981 --- /dev/null +++ b/internal/file/read_only_plugin.go @@ -0,0 +1,158 @@ +// Copyright (c) F5, Inc. +// +// This source code is licensed under the Apache License, Version 2.0 license found in the +// LICENSE file in the root directory of this source tree. + +package file + +import ( + "context" + "log/slog" + + "github.com/nginx/agent/v3/pkg/id" + + mpi "github.com/nginx/agent/v3/api/grpc/mpi/v1" + "github.com/nginx/agent/v3/internal/bus" + "github.com/nginx/agent/v3/internal/config" + "github.com/nginx/agent/v3/internal/grpc" + "github.com/nginx/agent/v3/internal/logger" + "github.com/nginx/agent/v3/internal/model" + "google.golang.org/protobuf/types/known/timestamppb" +) + +var _ bus.Plugin = (*ReadFilePlugin)(nil) + +// The file plugin only writes, deletes and checks hashes of files +// the file plugin does not care about the instance type + +type ReadFilePlugin struct { + messagePipe bus.MessagePipeInterface + config *config.Config + conn grpc.GrpcConnectionInterface + fileManagerService fileManagerServiceInterface +} + +func NewReadFilePlugin(agentConfig *config.Config, grpcConnection grpc.GrpcConnectionInterface) *ReadFilePlugin { + return &ReadFilePlugin{ + config: agentConfig, + conn: grpcConnection, + } +} + +func (rp *ReadFilePlugin) Init(ctx context.Context, messagePipe bus.MessagePipeInterface) error { + slog.DebugContext(ctx, "Starting read file plugin") + + rp.messagePipe = messagePipe + rp.fileManagerService = NewFileManagerService(rp.conn.FileServiceClient(), rp.config) + + return nil +} + +func (rp *ReadFilePlugin) Close(ctx context.Context) error { + slog.InfoContext(ctx, "Closing read file plugin") + return rp.conn.Close(ctx) +} + +func (rp *ReadFilePlugin) Info() *bus.Info { + return &bus.Info{ + Name: "read", + } +} + +func (rp *ReadFilePlugin) Process(ctx context.Context, msg *bus.Message) { + if logger.ServerType(ctx) == "auxiliary" || logger.ServerType(ctx) == "" { + switch msg.Topic { + case bus.ConnectionResetTopic: + rp.handleConnectionReset(ctx, msg) + case bus.ConnectionCreatedTopic: + slog.InfoContext(ctx, "Read file plugin received connection created message") + rp.fileManagerService.SetIsConnected(true) + case bus.NginxConfigUpdateTopic: + rp.handleNginxConfigUpdate(ctx, msg) + case bus.ConfigUploadRequestTopic: + rp.handleConfigUploadRequest(ctx, msg) + default: + slog.DebugContext(ctx, "Read file plugin received unknown topic", "topic", msg.Topic) + } + } +} + +func (rp *ReadFilePlugin) Subscriptions() []string { + return []string{ + bus.ConnectionResetTopic, + bus.ConnectionCreatedTopic, + bus.NginxConfigUpdateTopic, + bus.ConfigUploadRequestTopic, + } +} + +func (rp *ReadFilePlugin) handleConnectionReset(ctx context.Context, msg *bus.Message) { + slog.DebugContext(ctx, "Read file plugin received connection reset message") + if newConnection, ok := msg.Data.(grpc.GrpcConnectionInterface); ok { + var reconnect bool + err := rp.conn.Close(ctx) + if err != nil { + slog.ErrorContext(ctx, "Read file plugin unable to close connection", "error", err) + } + rp.conn = newConnection + + reconnect = rp.fileManagerService.IsConnected() + rp.fileManagerService = NewFileManagerService(rp.conn.FileServiceClient(), rp.config) + rp.fileManagerService.SetIsConnected(reconnect) + + slog.DebugContext(ctx, "File manager service client reset successfully") + } +} + +func (rp *ReadFilePlugin) handleNginxConfigUpdate(ctx context.Context, msg *bus.Message) { + slog.DebugContext(ctx, "Read file plugin received nginx config update message") + nginxConfigContext, ok := msg.Data.(*model.NginxConfigContext) + if !ok { + slog.ErrorContext(ctx, "Unable to cast message payload to *model.NginxConfigContext", "payload", msg.Data) + + return + } + + rp.fileManagerService.ConfigUpdate(ctx, nginxConfigContext) +} + +// nolint: dupl +func (rp *ReadFilePlugin) handleConfigUploadRequest(ctx context.Context, msg *bus.Message) { + slog.DebugContext(ctx, "Read file plugin received config upload request message") + managementPlaneRequest, ok := msg.Data.(*mpi.ManagementPlaneRequest) + if !ok { + slog.ErrorContext( + ctx, + "Unable to cast message payload to *mpi.ManagementPlaneRequest", + "payload", msg.Data, + ) + + return + } + + configUploadRequest := managementPlaneRequest.GetConfigUploadRequest() + + correlationID := logger.CorrelationID(ctx) + + updatingFilesError := rp.fileManagerService.ConfigUpload(ctx, configUploadRequest) + + response := &mpi.DataPlaneResponse{ + MessageMeta: &mpi.MessageMeta{ + MessageId: id.GenerateMessageID(), + CorrelationId: correlationID, + Timestamp: timestamppb.Now(), + }, + CommandResponse: &mpi.CommandResponse{ + Status: mpi.CommandResponse_COMMAND_STATUS_OK, + Message: "Successfully updated all files", + }, + } + + if updatingFilesError != nil { + response.CommandResponse.Status = mpi.CommandResponse_COMMAND_STATUS_FAILURE + response.CommandResponse.Message = "Failed to update all files" + response.CommandResponse.Error = updatingFilesError.Error() + } + + rp.messagePipe.Process(ctx, &bus.Message{Topic: bus.DataPlaneResponseTopic, Data: response}) +} diff --git a/internal/file/read_only_plugin_test.go b/internal/file/read_only_plugin_test.go new file mode 100644 index 000000000..5c6206d6a --- /dev/null +++ b/internal/file/read_only_plugin_test.go @@ -0,0 +1,247 @@ +// Copyright (c) F5, Inc. +// +// This source code is licensed under the Apache License, Version 2.0 license found in the +// LICENSE file in the root directory of this source tree. + +package file + +import ( + "context" + "os" + "testing" + "time" + + mpi "github.com/nginx/agent/v3/api/grpc/mpi/v1" + "github.com/nginx/agent/v3/api/grpc/mpi/v1/v1fakes" + "github.com/nginx/agent/v3/internal/bus" + "github.com/nginx/agent/v3/internal/bus/busfakes" + "github.com/nginx/agent/v3/internal/grpc/grpcfakes" + "github.com/nginx/agent/v3/internal/model" + "github.com/nginx/agent/v3/test/helpers" + "github.com/nginx/agent/v3/test/protos" + "github.com/nginx/agent/v3/test/types" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// nolint: dupl +func TestReadOnlyPlugin_Process_NginxConfigUpdateTopic(t *testing.T) { + ctx := context.Background() + + fileMeta := protos.FileMeta("/etc/nginx/nginx/conf", "") + + message := &model.NginxConfigContext{ + Files: []*mpi.File{ + { + FileMeta: fileMeta, + }, + }, + } + + fakeFileServiceClient := &v1fakes.FakeFileServiceClient{} + fakeFileServiceClient.UpdateOverviewReturns(&mpi.UpdateOverviewResponse{ + Overview: nil, + }, nil) + + fakeGrpcConnection := &grpcfakes.FakeGrpcConnectionInterface{} + fakeGrpcConnection.FileServiceClientReturns(fakeFileServiceClient) + messagePipe := busfakes.NewFakeMessagePipe() + + readPlugin := NewReadFilePlugin(types.AgentConfig(), fakeGrpcConnection) + err := readPlugin.Init(ctx, messagePipe) + require.NoError(t, err) + + readPlugin.Process(ctx, &bus.Message{Topic: bus.ConnectionCreatedTopic}) + readPlugin.Process(ctx, &bus.Message{Topic: bus.NginxConfigUpdateTopic, Data: message}) + + assert.Eventually( + t, + func() bool { return fakeFileServiceClient.UpdateOverviewCallCount() == 1 }, + 2*time.Second, + 10*time.Millisecond, + ) +} + +func TestReadPlugin_ConnectionReset(t *testing.T) { + ctx := context.Background() + + fakeFileServiceClient := &v1fakes.FakeFileServiceClient{} + fakeGrpcConnection := &grpcfakes.FakeGrpcConnectionInterface{} + fakeGrpcConnection.FileServiceClientReturns(fakeFileServiceClient) + + newFakeFileServiceClient := &v1fakes.FakeFileServiceClient{} + newGrpcConnection := &grpcfakes.FakeGrpcConnectionInterface{} + newGrpcConnection.FileServiceClientReturns(newFakeFileServiceClient) + + messagePipe := busfakes.NewFakeMessagePipe() + + readPlugin := NewReadFilePlugin(types.AgentConfig(), fakeGrpcConnection) + err := readPlugin.Init(ctx, messagePipe) + require.NoError(t, err) + + message := &bus.Message{Topic: bus.ConnectionResetTopic, Data: newGrpcConnection} + + readPlugin.Process(ctx, message) + assert.Eventually(t, + func() bool { + return fakeGrpcConnection.CloseCallCount() == 1 + }, + 2*time.Second, + 10*time.Millisecond, + ) + + assert.Eventually(t, + func() bool { + return newGrpcConnection.FileServiceClientCallCount() == 1 + }, + 2*time.Second, + 10*time.Millisecond, + ) + + assert.Equal(t, newGrpcConnection, readPlugin.conn) +} + +// nolint: dupl +func TestReadPlugin_Process_ConfigUploadRequestTopic(t *testing.T) { + ctx := context.Background() + + tempDir := os.TempDir() + testFile := helpers.CreateFileWithErrorCheck(t, tempDir, "nginx.conf") + defer helpers.RemoveFileWithErrorCheck(t, testFile.Name()) + fileMeta := protos.FileMeta(testFile.Name(), "") + + message := &mpi.ManagementPlaneRequest{ + Request: &mpi.ManagementPlaneRequest_ConfigUploadRequest{ + ConfigUploadRequest: &mpi.ConfigUploadRequest{ + Overview: &mpi.FileOverview{ + Files: []*mpi.File{ + { + FileMeta: fileMeta, + }, + { + FileMeta: fileMeta, + }, + }, + ConfigVersion: &mpi.ConfigVersion{ + InstanceId: "123", + Version: "f33ref3d32d3c32d3a", + }, + }, + }, + }, + } + + fakeFileServiceClient := &v1fakes.FakeFileServiceClient{} + fakeGrpcConnection := &grpcfakes.FakeGrpcConnectionInterface{} + fakeGrpcConnection.FileServiceClientReturns(fakeFileServiceClient) + messagePipe := busfakes.NewFakeMessagePipe() + + readPlugin := NewReadFilePlugin(types.AgentConfig(), fakeGrpcConnection) + err := readPlugin.Init(ctx, messagePipe) + require.NoError(t, err) + + readPlugin.Process(ctx, &bus.Message{Topic: bus.ConnectionCreatedTopic}) + readPlugin.Process(ctx, &bus.Message{Topic: bus.ConfigUploadRequestTopic, Data: message}) + + assert.Eventually( + t, + func() bool { return fakeFileServiceClient.UpdateFileCallCount() == 2 }, + 2*time.Second, + 10*time.Millisecond, + ) + + messages := messagePipe.Messages() + assert.Len(t, messages, 1) + assert.Equal(t, bus.DataPlaneResponseTopic, messages[0].Topic) + + dataPlaneResponse, ok := messages[0].Data.(*mpi.DataPlaneResponse) + assert.True(t, ok) + assert.Equal( + t, + mpi.CommandResponse_COMMAND_STATUS_OK, + dataPlaneResponse.GetCommandResponse().GetStatus(), + ) +} + +func TestReadPlugin_Process_ConfigUploadRequestTopic_Failure(t *testing.T) { + ctx := context.Background() + + fileMeta := protos.FileMeta("/unknown/file.conf", "") + + message := &mpi.ManagementPlaneRequest{ + Request: &mpi.ManagementPlaneRequest_ConfigUploadRequest{ + ConfigUploadRequest: &mpi.ConfigUploadRequest{ + Overview: &mpi.FileOverview{ + Files: []*mpi.File{ + { + FileMeta: fileMeta, + }, + { + FileMeta: fileMeta, + }, + }, + ConfigVersion: protos.CreateConfigVersion(), + }, + }, + }, + } + + fakeFileServiceClient := &v1fakes.FakeFileServiceClient{} + fakeGrpcConnection := &grpcfakes.FakeGrpcConnectionInterface{} + fakeGrpcConnection.FileServiceClientReturns(fakeFileServiceClient) + messagePipe := busfakes.NewFakeMessagePipe() + + readPlugin := NewReadFilePlugin(types.AgentConfig(), fakeGrpcConnection) + err := readPlugin.Init(ctx, messagePipe) + require.NoError(t, err) + + readPlugin.Process(ctx, &bus.Message{Topic: bus.ConnectionCreatedTopic}) + readPlugin.Process(ctx, &bus.Message{Topic: bus.ConfigUploadRequestTopic, Data: message}) + + assert.Eventually( + t, + func() bool { return len(messagePipe.Messages()) == 1 }, + 2*time.Second, + 10*time.Millisecond, + ) + + assert.Equal(t, 0, fakeFileServiceClient.UpdateFileCallCount()) + + messages := messagePipe.Messages() + assert.Len(t, messages, 1) + + assert.Equal(t, bus.DataPlaneResponseTopic, messages[0].Topic) + + dataPlaneResponse, ok := messages[0].Data.(*mpi.DataPlaneResponse) + assert.True(t, ok) + assert.Equal( + t, + mpi.CommandResponse_COMMAND_STATUS_FAILURE, + dataPlaneResponse.GetCommandResponse().GetStatus(), + ) +} + +func TestReadPlugin_Subscriptions(t *testing.T) { + readPlugin := NewReadFilePlugin(types.AgentConfig(), &grpcfakes.FakeGrpcConnectionInterface{}) + assert.Equal(t, []string{ + bus.ConnectionResetTopic, + bus.ConnectionCreatedTopic, + bus.NginxConfigUpdateTopic, + bus.ConfigUploadRequestTopic, + }, readPlugin.Subscriptions()) +} + +func TestReadPlugin_Info(t *testing.T) { + filePlugin := NewReadFilePlugin(types.AgentConfig(), &grpcfakes.FakeGrpcConnectionInterface{}) + assert.Equal(t, "read", filePlugin.Info().Name) +} + +func TestReadPlugin_Close(t *testing.T) { + ctx := context.Background() + fakeGrpcConnection := &grpcfakes.FakeGrpcConnectionInterface{} + + filePlugin := NewReadFilePlugin(types.AgentConfig(), fakeGrpcConnection) + filePlugin.Close(ctx) + + assert.Equal(t, 1, fakeGrpcConnection.CloseCallCount()) +} diff --git a/internal/plugin/plugin_manager.go b/internal/plugin/plugin_manager.go index e81aa2101..d1eeab8c7 100644 --- a/internal/plugin/plugin_manager.go +++ b/internal/plugin/plugin_manager.go @@ -70,7 +70,8 @@ func addAuxiliaryCommandAndFilePlugins(ctx context.Context, plugins []bus.Plugin } else { auxCommandPlugin := command.NewCommandPlugin(agentConfig, auxGRPCConnection, "auxiliary") plugins = append(plugins, auxCommandPlugin) - // Followup PR to add read plugin eventually + readFilePlugin := file.NewReadFilePlugin(agentConfig, auxGRPCConnection) + plugins = append(plugins, readFilePlugin) } } else { slog.InfoContext(ctx, "Agent is not connected to an auxiliary management plane. "+ diff --git a/internal/plugin/plugin_manager_test.go b/internal/plugin/plugin_manager_test.go index ff0dca6d0..bf73936d1 100644 --- a/internal/plugin/plugin_manager_test.go +++ b/internal/plugin/plugin_manager_test.go @@ -48,12 +48,21 @@ func TestLoadPlugins(t *testing.T) { Type: config.Grpc, }, }, + AuxiliaryCommand: &config.Command{ + Server: &config.ServerConfig{ + Host: "test.connect", + Port: 443, + Type: config.Grpc, + }, + }, Features: config.DefaultFeatures(), }, expected: []bus.Plugin{ &resource.Resource{}, &command.CommandPlugin{}, &file.FilePlugin{}, + &command.CommandPlugin{}, + &file.ReadFilePlugin{}, &watcher.Watcher{}, }, }, From 650a0d97a0ef1c333aad4241abee6591a16212a4 Mon Sep 17 00:00:00 2001 From: Aphral Griffin Date: Mon, 16 Jun 2025 15:25:07 +0100 Subject: [PATCH 04/17] PR feedback --- internal/command/command_plugin.go | 67 ++++++++++++++++++------- internal/command/command_plugin_test.go | 20 ++++---- internal/command/command_service.go | 2 +- internal/logger/logger.go | 1 + internal/plugin/plugin_manager.go | 6 +-- 5 files changed, 62 insertions(+), 34 deletions(-) diff --git a/internal/command/command_plugin.go b/internal/command/command_plugin.go index ab9b624ff..84e410b34 100644 --- a/internal/command/command_plugin.go +++ b/internal/command/command_plugin.go @@ -25,6 +25,17 @@ var _ bus.Plugin = (*CommandPlugin)(nil) //go:generate go run github.com/maxbrunsfeld/counterfeiter/v6@v6.8.1 -generate //counterfeiter:generate . commandService +type ServerType int + +const ( + Command ServerType = iota + Auxiliary +) + +var serverType = map[ServerType]string{ + Command: "command", + Auxiliary: "auxiliary", +} type ( commandService interface { @@ -44,13 +55,13 @@ type ( conn grpc.GrpcConnectionInterface commandService commandService subscribeChannel chan *mpi.ManagementPlaneRequest - commandServerType string + commandServerType ServerType subscribeMutex sync.Mutex } ) func NewCommandPlugin(agentConfig *config.Config, grpcConnection grpc.GrpcConnectionInterface, - commandServerType string, + commandServerType ServerType, ) *CommandPlugin { return &CommandPlugin{ config: agentConfig, @@ -61,22 +72,22 @@ func NewCommandPlugin(agentConfig *config.Config, grpcConnection grpc.GrpcConnec } func (cp *CommandPlugin) Init(ctx context.Context, messagePipe bus.MessagePipeInterface) error { - slog.DebugContext(ctx, "Starting command plugin", "command_server_type", cp.commandServerType) + newCtx := context.WithValue( + ctx, + logger.ServerTypeContextKey, slog.Any(logger.ServerTypeKey, cp.commandServerType.String()), + ) + slog.DebugContext(newCtx, "Starting command plugin", "command_server_type", cp.commandServerType.String()) cp.messagePipe = messagePipe cp.commandService = NewCommandService(cp.conn.CommandServiceClient(), cp.config, cp.subscribeChannel) - newCtx := context.WithValue( - ctx, - logger.ServerTypeContextKey, slog.Any(logger.ServerTypeKey, cp.commandServerType), - ) go cp.monitorSubscribeChannel(newCtx) return nil } func (cp *CommandPlugin) Close(ctx context.Context) error { - slog.InfoContext(ctx, "Closing command plugin", "command_server_type", cp.commandServerType) + slog.InfoContext(ctx, "Closing command plugin", "command_server_type", cp.commandServerType.String()) cp.subscribeMutex.Lock() if cp.subscribeCancel != nil { @@ -89,14 +100,14 @@ func (cp *CommandPlugin) Close(ctx context.Context) error { func (cp *CommandPlugin) Info() *bus.Info { return &bus.Info{ - Name: cp.commandServerType, + Name: cp.commandServerType.String(), } } func (cp *CommandPlugin) Process(ctx context.Context, msg *bus.Message) { slog.DebugContext(ctx, "Processing command", "command_server_type", logger.ServerType(ctx)) - if logger.ServerType(ctx) == cp.commandServerType || logger.ServerType(ctx) == "" { + if logger.ServerType(ctx) == cp.commandServerType.String() || logger.ServerType(ctx) == "" { switch msg.Topic { case bus.ConnectionResetTopic: cp.processConnectionReset(ctx, msg) @@ -111,9 +122,6 @@ func (cp *CommandPlugin) Process(ctx context.Context, msg *bus.Message) { default: slog.DebugContext(ctx, "Command plugin received unknown topic", "topic", msg.Topic) } - } else { - slog.Info("Sever type is not right ignoring message", "command_server_type", - logger.ServerType(ctx), "topic", msg.Topic) } } @@ -123,7 +131,7 @@ func (cp *CommandPlugin) processResourceUpdate(ctx context.Context, msg *bus.Mes if !cp.commandService.IsConnected() { newCtx := context.WithValue( ctx, - logger.ServerTypeContextKey, slog.Any(logger.ServerTypeKey, cp.commandServerType), + logger.ServerTypeContextKey, slog.Any(logger.ServerTypeKey, cp.commandServerType.String()), ) cp.createConnection(newCtx, resource) } else { @@ -185,7 +193,7 @@ func (cp *CommandPlugin) processInstanceHealth(ctx context.Context, msg *bus.Mes err := cp.commandService.UpdateDataPlaneHealth(ctx, instances) if err != nil { slog.ErrorContext(ctx, "Unable to update data plane health", "error", err, - "command_server_type", cp.commandServerType) + "command_server_type", cp.commandServerType.String()) } } } @@ -248,9 +256,10 @@ func (cp *CommandPlugin) monitorSubscribeChannel(ctx context.Context) { slog.InfoContext(ctx, "Received management plane config upload request") cp.handleConfigUploadRequest(newCtx, message) case *mpi.ManagementPlaneRequest_ConfigApplyRequest: - if cp.commandServerType != "command" { + if cp.commandServerType != Command { slog.WarnContext(newCtx, "Auxiliary command server can not perform config apply", - "command_server_type", cp.commandServerType) + "command_server_type", cp.commandServerType.String()) + cp.handleInvalidRequest(newCtx, message) return } @@ -260,9 +269,10 @@ func (cp *CommandPlugin) monitorSubscribeChannel(ctx context.Context) { slog.InfoContext(ctx, "Received management plane health request") cp.handleHealthRequest(newCtx) case *mpi.ManagementPlaneRequest_ActionRequest: - if cp.commandServerType != "command" { + if cp.commandServerType != Command { slog.WarnContext(newCtx, "Auxiliary command server can not perform api action", - "command_server_type", cp.commandServerType) + "command_server_type", cp.commandServerType.String()) + cp.handleInvalidRequest(newCtx, message) return } @@ -354,6 +364,21 @@ func (cp *CommandPlugin) handleHealthRequest(newCtx context.Context) { cp.messagePipe.Process(newCtx, &bus.Message{Topic: bus.DataPlaneHealthRequestTopic}) } +func (cp *CommandPlugin) handleInvalidRequest(ctx context.Context, message *mpi.ManagementPlaneRequest) { + err := cp.commandService.SendDataPlaneResponse(ctx, &mpi.DataPlaneResponse{ + MessageMeta: message.GetMessageMeta(), + CommandResponse: &mpi.CommandResponse{ + Status: mpi.CommandResponse_COMMAND_STATUS_FAILURE, + Message: "Can not perform write action as auxiliary command server", + Error: "request not allowed", + }, + InstanceId: message.GetActionRequest().GetInstanceId(), + }) + if err != nil { + slog.ErrorContext(ctx, "Unable to send data plane response", "error", err) + } +} + func (cp *CommandPlugin) createDataPlaneResponse(correlationID string, status mpi.CommandResponse_CommandStatus, message, err string, ) *mpi.DataPlaneResponse { @@ -370,3 +395,7 @@ func (cp *CommandPlugin) createDataPlaneResponse(correlationID string, status mp }, } } + +func (s ServerType) String() string { + return serverType[s] +} diff --git a/internal/command/command_plugin_test.go b/internal/command/command_plugin_test.go index 9048e4873..7ebea0fef 100644 --- a/internal/command/command_plugin_test.go +++ b/internal/command/command_plugin_test.go @@ -31,17 +31,15 @@ import ( "github.com/stretchr/testify/require" ) -const serverType = "command" - func TestCommandPlugin_Info(t *testing.T) { - commandPlugin := NewCommandPlugin(types.AgentConfig(), &grpcfakes.FakeGrpcConnectionInterface{}, serverType) + commandPlugin := NewCommandPlugin(types.AgentConfig(), &grpcfakes.FakeGrpcConnectionInterface{}, Command) info := commandPlugin.Info() assert.Equal(t, "command", info.Name) } func TestCommandPlugin_Subscriptions(t *testing.T) { - commandPlugin := NewCommandPlugin(types.AgentConfig(), &grpcfakes.FakeGrpcConnectionInterface{}, serverType) + commandPlugin := NewCommandPlugin(types.AgentConfig(), &grpcfakes.FakeGrpcConnectionInterface{}, Command) subscriptions := commandPlugin.Subscriptions() assert.Equal( @@ -62,7 +60,7 @@ func TestCommandPlugin_Init(t *testing.T) { messagePipe := busfakes.NewFakeMessagePipe() fakeCommandService := &commandfakes.FakeCommandService{} - commandPlugin := NewCommandPlugin(types.AgentConfig(), &grpcfakes.FakeGrpcConnectionInterface{}, serverType) + commandPlugin := NewCommandPlugin(types.AgentConfig(), &grpcfakes.FakeGrpcConnectionInterface{}, Command) err := commandPlugin.Init(ctx, messagePipe) require.NoError(t, err) @@ -81,7 +79,7 @@ func TestCommandPlugin_createConnection(t *testing.T) { commandService.CreateConnectionReturns(&mpi.CreateConnectionResponse{}, nil) messagePipe := busfakes.NewFakeMessagePipe() - commandPlugin := NewCommandPlugin(types.AgentConfig(), &grpcfakes.FakeGrpcConnectionInterface{}, serverType) + commandPlugin := NewCommandPlugin(types.AgentConfig(), &grpcfakes.FakeGrpcConnectionInterface{}, Command) err := commandPlugin.Init(ctx, messagePipe) commandPlugin.commandService = commandService require.NoError(t, err) @@ -113,7 +111,7 @@ func TestCommandPlugin_Process(t *testing.T) { messagePipe := busfakes.NewFakeMessagePipe() fakeCommandService := &commandfakes.FakeCommandService{} - commandPlugin := NewCommandPlugin(types.AgentConfig(), &grpcfakes.FakeGrpcConnectionInterface{}, serverType) + commandPlugin := NewCommandPlugin(types.AgentConfig(), &grpcfakes.FakeGrpcConnectionInterface{}, Command) err := commandPlugin.Init(ctx, messagePipe) require.NoError(t, err) defer commandPlugin.Close(ctx) @@ -221,7 +219,7 @@ func TestCommandPlugin_monitorSubscribeChannel(t *testing.T) { agentConfig := types.AgentConfig() agentConfig.Features = test.configFeatures - commandPlugin := NewCommandPlugin(agentConfig, &grpcfakes.FakeGrpcConnectionInterface{}, serverType) + commandPlugin := NewCommandPlugin(agentConfig, &grpcfakes.FakeGrpcConnectionInterface{}, Command) err := commandPlugin.Init(ctx, messagePipe) require.NoError(tt, err) defer commandPlugin.Close(ctx) @@ -321,7 +319,7 @@ func TestCommandPlugin_FeatureDisabled(t *testing.T) { agentConfig.Features = test.configFeatures - commandPlugin := NewCommandPlugin(agentConfig, &grpcfakes.FakeGrpcConnectionInterface{}, serverType) + commandPlugin := NewCommandPlugin(agentConfig, &grpcfakes.FakeGrpcConnectionInterface{}, Command) err := commandPlugin.Init(ctx, messagePipe) commandPlugin.commandService = fakeCommandService require.NoError(tt, err) @@ -346,7 +344,7 @@ func TestMonitorSubscribeChannel(t *testing.T) { logBuf := &bytes.Buffer{} stub.StubLoggerWith(logBuf) - cp := NewCommandPlugin(types.AgentConfig(), &grpcfakes.FakeGrpcConnectionInterface{}, serverType) + cp := NewCommandPlugin(types.AgentConfig(), &grpcfakes.FakeGrpcConnectionInterface{}, Command) cp.subscribeCancel = cncl message := protos.CreateManagementPlaneRequest() @@ -385,7 +383,7 @@ func Test_createDataPlaneResponse(t *testing.T) { Error: "", }, } - commandPlugin := NewCommandPlugin(types.AgentConfig(), &grpcfakes.FakeGrpcConnectionInterface{}, serverType) + commandPlugin := NewCommandPlugin(types.AgentConfig(), &grpcfakes.FakeGrpcConnectionInterface{}, Command) result := commandPlugin.createDataPlaneResponse(expected.GetMessageMeta().GetCorrelationId(), expected.GetCommandResponse().GetStatus(), expected.GetCommandResponse().GetMessage(), expected.GetCommandResponse().GetError()) diff --git a/internal/command/command_service.go b/internal/command/command_service.go index 4f4746790..15b35604c 100644 --- a/internal/command/command_service.go +++ b/internal/command/command_service.go @@ -261,7 +261,7 @@ func (cs *CommandService) UpdateClient(ctx context.Context, client mpi.CommandSe cs.subscribeClientMutex.Unlock() cs.isConnected.Store(false) - // Will this have the sever type ? + resp, err := cs.CreateConnection(ctx, cs.resource) if err != nil { return err diff --git a/internal/logger/logger.go b/internal/logger/logger.go index 4f903ea02..aa6f66f2a 100644 --- a/internal/logger/logger.go +++ b/internal/logger/logger.go @@ -75,6 +75,7 @@ func New(logPath, level string) *slog.Logger { contextHandler{ handler, []any{ CorrelationIDContextKey, + ServerTypeContextKey, }, }) } diff --git a/internal/plugin/plugin_manager.go b/internal/plugin/plugin_manager.go index e81aa2101..31f3c81f1 100644 --- a/internal/plugin/plugin_manager.go +++ b/internal/plugin/plugin_manager.go @@ -47,7 +47,7 @@ func addCommandAndFilePlugins(ctx context.Context, plugins []bus.Plugin, agentCo if err != nil { slog.WarnContext(ctx, "Failed to create gRPC connection for command server", "error", err) } else { - commandPlugin := command.NewCommandPlugin(agentConfig, grpcConnection, "command") + commandPlugin := command.NewCommandPlugin(agentConfig, grpcConnection, command.Command) plugins = append(plugins, commandPlugin) filePlugin := file.NewFilePlugin(agentConfig, grpcConnection) plugins = append(plugins, filePlugin) @@ -68,12 +68,12 @@ func addAuxiliaryCommandAndFilePlugins(ctx context.Context, plugins []bus.Plugin if err != nil { slog.WarnContext(ctx, "Failed to create gRPC connection for auxiliary command server", "error", err) } else { - auxCommandPlugin := command.NewCommandPlugin(agentConfig, auxGRPCConnection, "auxiliary") + auxCommandPlugin := command.NewCommandPlugin(agentConfig, auxGRPCConnection, command.Auxiliary) plugins = append(plugins, auxCommandPlugin) // Followup PR to add read plugin eventually } } else { - slog.InfoContext(ctx, "Agent is not connected to an auxiliary management plane. "+ + slog.DebugContext(ctx, "Agent is not connected to an auxiliary management plane. "+ "Configure a auxiliary command server to establish a connection.") } From 8ef9c26e65831ca6e62a39a1f9d7ffaf67135daf Mon Sep 17 00:00:00 2001 From: Aphral Griffin Date: Mon, 16 Jun 2025 16:16:51 +0100 Subject: [PATCH 05/17] file plugin --- internal/file/file_plugin.go | 44 +++++++++++++++++-------------- internal/file/read_only_plugin.go | 3 ++- 2 files changed, 26 insertions(+), 21 deletions(-) diff --git a/internal/file/file_plugin.go b/internal/file/file_plugin.go index 67e2138ed..98b45d85d 100644 --- a/internal/file/file_plugin.go +++ b/internal/file/file_plugin.go @@ -7,6 +7,7 @@ package file import ( "context" + "github.com/nginx/agent/v3/internal/command" "log/slog" "github.com/nginx/agent/v3/pkg/files" @@ -61,27 +62,30 @@ func (fp *FilePlugin) Info() *bus.Info { } func (fp *FilePlugin) Process(ctx context.Context, msg *bus.Message) { - switch msg.Topic { - case bus.ConnectionResetTopic: - fp.handleConnectionReset(ctx, msg) - case bus.ConnectionCreatedTopic: - slog.DebugContext(ctx, "File plugin received connection created message") - fp.fileManagerService.SetIsConnected(true) - case bus.NginxConfigUpdateTopic: - fp.handleNginxConfigUpdate(ctx, msg) - case bus.ConfigUploadRequestTopic: - fp.handleConfigUploadRequest(ctx, msg) - case bus.ConfigApplyRequestTopic: - fp.handleConfigApplyRequest(ctx, msg) - case bus.ConfigApplyCompleteTopic: - fp.handleConfigApplyComplete(ctx, msg) - case bus.ConfigApplySuccessfulTopic: - fp.handleConfigApplySuccess(ctx, msg) - case bus.ConfigApplyFailedTopic: - fp.handleConfigApplyFailedRequest(ctx, msg) - default: - slog.DebugContext(ctx, "File plugin received unknown topic", "topic", msg.Topic) + if logger.ServerType(ctx) == command.Command.String() || logger.ServerType(ctx) == "" { + switch msg.Topic { + case bus.ConnectionResetTopic: + fp.handleConnectionReset(ctx, msg) + case bus.ConnectionCreatedTopic: + slog.DebugContext(ctx, "File plugin received connection created message") + fp.fileManagerService.SetIsConnected(true) + case bus.NginxConfigUpdateTopic: + fp.handleNginxConfigUpdate(ctx, msg) + case bus.ConfigUploadRequestTopic: + fp.handleConfigUploadRequest(ctx, msg) + case bus.ConfigApplyRequestTopic: + fp.handleConfigApplyRequest(ctx, msg) + case bus.ConfigApplyCompleteTopic: + fp.handleConfigApplyComplete(ctx, msg) + case bus.ConfigApplySuccessfulTopic: + fp.handleConfigApplySuccess(ctx, msg) + case bus.ConfigApplyFailedTopic: + fp.handleConfigApplyFailedRequest(ctx, msg) + default: + slog.DebugContext(ctx, "File plugin received unknown topic", "topic", msg.Topic) + } } + } func (fp *FilePlugin) Subscriptions() []string { diff --git a/internal/file/read_only_plugin.go b/internal/file/read_only_plugin.go index fda214981..b7f48043b 100644 --- a/internal/file/read_only_plugin.go +++ b/internal/file/read_only_plugin.go @@ -7,6 +7,7 @@ package file import ( "context" + "github.com/nginx/agent/v3/internal/command" "log/slog" "github.com/nginx/agent/v3/pkg/id" @@ -60,7 +61,7 @@ func (rp *ReadFilePlugin) Info() *bus.Info { } func (rp *ReadFilePlugin) Process(ctx context.Context, msg *bus.Message) { - if logger.ServerType(ctx) == "auxiliary" || logger.ServerType(ctx) == "" { + if logger.ServerType(ctx) == command.Auxiliary.String() || logger.ServerType(ctx) == "" { switch msg.Topic { case bus.ConnectionResetTopic: rp.handleConnectionReset(ctx, msg) From a8817abc38179c64c35a0af559872472ffce2dc7 Mon Sep 17 00:00:00 2001 From: Aphral Griffin Date: Mon, 16 Jun 2025 16:30:01 +0100 Subject: [PATCH 06/17] file plugin --- internal/file/file_plugin.go | 5 +++-- internal/file/read_only_plugin.go | 3 ++- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/internal/file/file_plugin.go b/internal/file/file_plugin.go index 98b45d85d..e52b6a734 100644 --- a/internal/file/file_plugin.go +++ b/internal/file/file_plugin.go @@ -7,9 +7,10 @@ package file import ( "context" - "github.com/nginx/agent/v3/internal/command" "log/slog" + "github.com/nginx/agent/v3/internal/command" + "github.com/nginx/agent/v3/pkg/files" "github.com/nginx/agent/v3/pkg/id" @@ -61,6 +62,7 @@ func (fp *FilePlugin) Info() *bus.Info { } } +// nolint: cyclop, revive func (fp *FilePlugin) Process(ctx context.Context, msg *bus.Message) { if logger.ServerType(ctx) == command.Command.String() || logger.ServerType(ctx) == "" { switch msg.Topic { @@ -85,7 +87,6 @@ func (fp *FilePlugin) Process(ctx context.Context, msg *bus.Message) { slog.DebugContext(ctx, "File plugin received unknown topic", "topic", msg.Topic) } } - } func (fp *FilePlugin) Subscriptions() []string { diff --git a/internal/file/read_only_plugin.go b/internal/file/read_only_plugin.go index b7f48043b..c9af019b5 100644 --- a/internal/file/read_only_plugin.go +++ b/internal/file/read_only_plugin.go @@ -7,9 +7,10 @@ package file import ( "context" - "github.com/nginx/agent/v3/internal/command" "log/slog" + "github.com/nginx/agent/v3/internal/command" + "github.com/nginx/agent/v3/pkg/id" mpi "github.com/nginx/agent/v3/api/grpc/mpi/v1" From b7108550f81225d7d60d6e9ad4e652dcf3f2fc23 Mon Sep 17 00:00:00 2001 From: Aphral Griffin Date: Mon, 23 Jun 2025 11:18:36 +0100 Subject: [PATCH 07/17] PR feedback --- internal/command/command_plugin.go | 25 ++++++------------------- internal/command/command_plugin_test.go | 20 +++++++++++--------- internal/model/server.go | 22 ++++++++++++++++++++++ internal/plugin/plugin_manager.go | 6 ++++-- 4 files changed, 43 insertions(+), 30 deletions(-) create mode 100644 internal/model/server.go diff --git a/internal/command/command_plugin.go b/internal/command/command_plugin.go index 84e410b34..d304934f3 100644 --- a/internal/command/command_plugin.go +++ b/internal/command/command_plugin.go @@ -10,6 +10,8 @@ import ( "log/slog" "sync" + "github.com/nginx/agent/v3/internal/model" + "google.golang.org/protobuf/types/known/timestamppb" mpi "github.com/nginx/agent/v3/api/grpc/mpi/v1" @@ -25,17 +27,6 @@ var _ bus.Plugin = (*CommandPlugin)(nil) //go:generate go run github.com/maxbrunsfeld/counterfeiter/v6@v6.8.1 -generate //counterfeiter:generate . commandService -type ServerType int - -const ( - Command ServerType = iota - Auxiliary -) - -var serverType = map[ServerType]string{ - Command: "command", - Auxiliary: "auxiliary", -} type ( commandService interface { @@ -55,13 +46,13 @@ type ( conn grpc.GrpcConnectionInterface commandService commandService subscribeChannel chan *mpi.ManagementPlaneRequest - commandServerType ServerType + commandServerType model.ServerType subscribeMutex sync.Mutex } ) func NewCommandPlugin(agentConfig *config.Config, grpcConnection grpc.GrpcConnectionInterface, - commandServerType ServerType, + commandServerType model.ServerType, ) *CommandPlugin { return &CommandPlugin{ config: agentConfig, @@ -256,7 +247,7 @@ func (cp *CommandPlugin) monitorSubscribeChannel(ctx context.Context) { slog.InfoContext(ctx, "Received management plane config upload request") cp.handleConfigUploadRequest(newCtx, message) case *mpi.ManagementPlaneRequest_ConfigApplyRequest: - if cp.commandServerType != Command { + if cp.commandServerType != model.Command { slog.WarnContext(newCtx, "Auxiliary command server can not perform config apply", "command_server_type", cp.commandServerType.String()) cp.handleInvalidRequest(newCtx, message) @@ -269,7 +260,7 @@ func (cp *CommandPlugin) monitorSubscribeChannel(ctx context.Context) { slog.InfoContext(ctx, "Received management plane health request") cp.handleHealthRequest(newCtx) case *mpi.ManagementPlaneRequest_ActionRequest: - if cp.commandServerType != Command { + if cp.commandServerType != model.Command { slog.WarnContext(newCtx, "Auxiliary command server can not perform api action", "command_server_type", cp.commandServerType.String()) cp.handleInvalidRequest(newCtx, message) @@ -395,7 +386,3 @@ func (cp *CommandPlugin) createDataPlaneResponse(correlationID string, status mp }, } } - -func (s ServerType) String() string { - return serverType[s] -} diff --git a/internal/command/command_plugin_test.go b/internal/command/command_plugin_test.go index 7ebea0fef..c51f3c579 100644 --- a/internal/command/command_plugin_test.go +++ b/internal/command/command_plugin_test.go @@ -11,6 +11,8 @@ import ( "testing" "time" + "github.com/nginx/agent/v3/internal/model" + pkg "github.com/nginx/agent/v3/pkg/config" "github.com/nginx/agent/v3/pkg/id" @@ -32,14 +34,14 @@ import ( ) func TestCommandPlugin_Info(t *testing.T) { - commandPlugin := NewCommandPlugin(types.AgentConfig(), &grpcfakes.FakeGrpcConnectionInterface{}, Command) + commandPlugin := NewCommandPlugin(types.AgentConfig(), &grpcfakes.FakeGrpcConnectionInterface{}, model.Command) info := commandPlugin.Info() assert.Equal(t, "command", info.Name) } func TestCommandPlugin_Subscriptions(t *testing.T) { - commandPlugin := NewCommandPlugin(types.AgentConfig(), &grpcfakes.FakeGrpcConnectionInterface{}, Command) + commandPlugin := NewCommandPlugin(types.AgentConfig(), &grpcfakes.FakeGrpcConnectionInterface{}, model.Command) subscriptions := commandPlugin.Subscriptions() assert.Equal( @@ -60,7 +62,7 @@ func TestCommandPlugin_Init(t *testing.T) { messagePipe := busfakes.NewFakeMessagePipe() fakeCommandService := &commandfakes.FakeCommandService{} - commandPlugin := NewCommandPlugin(types.AgentConfig(), &grpcfakes.FakeGrpcConnectionInterface{}, Command) + commandPlugin := NewCommandPlugin(types.AgentConfig(), &grpcfakes.FakeGrpcConnectionInterface{}, model.Command) err := commandPlugin.Init(ctx, messagePipe) require.NoError(t, err) @@ -79,7 +81,7 @@ func TestCommandPlugin_createConnection(t *testing.T) { commandService.CreateConnectionReturns(&mpi.CreateConnectionResponse{}, nil) messagePipe := busfakes.NewFakeMessagePipe() - commandPlugin := NewCommandPlugin(types.AgentConfig(), &grpcfakes.FakeGrpcConnectionInterface{}, Command) + commandPlugin := NewCommandPlugin(types.AgentConfig(), &grpcfakes.FakeGrpcConnectionInterface{}, model.Command) err := commandPlugin.Init(ctx, messagePipe) commandPlugin.commandService = commandService require.NoError(t, err) @@ -111,7 +113,7 @@ func TestCommandPlugin_Process(t *testing.T) { messagePipe := busfakes.NewFakeMessagePipe() fakeCommandService := &commandfakes.FakeCommandService{} - commandPlugin := NewCommandPlugin(types.AgentConfig(), &grpcfakes.FakeGrpcConnectionInterface{}, Command) + commandPlugin := NewCommandPlugin(types.AgentConfig(), &grpcfakes.FakeGrpcConnectionInterface{}, model.Command) err := commandPlugin.Init(ctx, messagePipe) require.NoError(t, err) defer commandPlugin.Close(ctx) @@ -219,7 +221,7 @@ func TestCommandPlugin_monitorSubscribeChannel(t *testing.T) { agentConfig := types.AgentConfig() agentConfig.Features = test.configFeatures - commandPlugin := NewCommandPlugin(agentConfig, &grpcfakes.FakeGrpcConnectionInterface{}, Command) + commandPlugin := NewCommandPlugin(agentConfig, &grpcfakes.FakeGrpcConnectionInterface{}, model.Command) err := commandPlugin.Init(ctx, messagePipe) require.NoError(tt, err) defer commandPlugin.Close(ctx) @@ -319,7 +321,7 @@ func TestCommandPlugin_FeatureDisabled(t *testing.T) { agentConfig.Features = test.configFeatures - commandPlugin := NewCommandPlugin(agentConfig, &grpcfakes.FakeGrpcConnectionInterface{}, Command) + commandPlugin := NewCommandPlugin(agentConfig, &grpcfakes.FakeGrpcConnectionInterface{}, model.Command) err := commandPlugin.Init(ctx, messagePipe) commandPlugin.commandService = fakeCommandService require.NoError(tt, err) @@ -344,7 +346,7 @@ func TestMonitorSubscribeChannel(t *testing.T) { logBuf := &bytes.Buffer{} stub.StubLoggerWith(logBuf) - cp := NewCommandPlugin(types.AgentConfig(), &grpcfakes.FakeGrpcConnectionInterface{}, Command) + cp := NewCommandPlugin(types.AgentConfig(), &grpcfakes.FakeGrpcConnectionInterface{}, model.Command) cp.subscribeCancel = cncl message := protos.CreateManagementPlaneRequest() @@ -383,7 +385,7 @@ func Test_createDataPlaneResponse(t *testing.T) { Error: "", }, } - commandPlugin := NewCommandPlugin(types.AgentConfig(), &grpcfakes.FakeGrpcConnectionInterface{}, Command) + commandPlugin := NewCommandPlugin(types.AgentConfig(), &grpcfakes.FakeGrpcConnectionInterface{}, model.Command) result := commandPlugin.createDataPlaneResponse(expected.GetMessageMeta().GetCorrelationId(), expected.GetCommandResponse().GetStatus(), expected.GetCommandResponse().GetMessage(), expected.GetCommandResponse().GetError()) diff --git a/internal/model/server.go b/internal/model/server.go new file mode 100644 index 000000000..b2f5607ff --- /dev/null +++ b/internal/model/server.go @@ -0,0 +1,22 @@ +// Copyright (c) F5, Inc. +// +// This source code is licensed under the Apache License, Version 2.0 license found in the +// LICENSE file in the root directory of this source tree. + +package model + +type ServerType int + +const ( + Command ServerType = iota + Auxiliary +) + +var serverType = map[ServerType]string{ + Command: "command", + Auxiliary: "auxiliary", +} + +func (s ServerType) String() string { + return serverType[s] +} diff --git a/internal/plugin/plugin_manager.go b/internal/plugin/plugin_manager.go index 31f3c81f1..96b0b1a86 100644 --- a/internal/plugin/plugin_manager.go +++ b/internal/plugin/plugin_manager.go @@ -9,6 +9,8 @@ import ( "context" "log/slog" + "github.com/nginx/agent/v3/internal/model" + pkg "github.com/nginx/agent/v3/pkg/config" "github.com/nginx/agent/v3/internal/collector" @@ -47,7 +49,7 @@ func addCommandAndFilePlugins(ctx context.Context, plugins []bus.Plugin, agentCo if err != nil { slog.WarnContext(ctx, "Failed to create gRPC connection for command server", "error", err) } else { - commandPlugin := command.NewCommandPlugin(agentConfig, grpcConnection, command.Command) + commandPlugin := command.NewCommandPlugin(agentConfig, grpcConnection, model.Command) plugins = append(plugins, commandPlugin) filePlugin := file.NewFilePlugin(agentConfig, grpcConnection) plugins = append(plugins, filePlugin) @@ -68,7 +70,7 @@ func addAuxiliaryCommandAndFilePlugins(ctx context.Context, plugins []bus.Plugin if err != nil { slog.WarnContext(ctx, "Failed to create gRPC connection for auxiliary command server", "error", err) } else { - auxCommandPlugin := command.NewCommandPlugin(agentConfig, auxGRPCConnection, command.Auxiliary) + auxCommandPlugin := command.NewCommandPlugin(agentConfig, auxGRPCConnection, model.Auxiliary) plugins = append(plugins, auxCommandPlugin) // Followup PR to add read plugin eventually } From 8ef3c4592534715b844b42c206f7205fef706f34 Mon Sep 17 00:00:00 2001 From: Aphral Griffin Date: Mon, 23 Jun 2025 14:40:43 +0100 Subject: [PATCH 08/17] refactor file plugin to have read only mode --- internal/file/file_plugin.go | 19 +- internal/file/file_plugin_test.go | 30 +-- internal/file/file_service_operator.go | 2 + internal/file/read_only_plugin.go | 158 ---------------- internal/file/read_only_plugin_test.go | 247 ------------------------- internal/plugin/plugin_manager.go | 4 +- internal/plugin/plugin_manager_test.go | 2 +- 7 files changed, 39 insertions(+), 423 deletions(-) delete mode 100644 internal/file/read_only_plugin.go delete mode 100644 internal/file/read_only_plugin_test.go diff --git a/internal/file/file_plugin.go b/internal/file/file_plugin.go index 8c3fcf45b..be99fffa6 100644 --- a/internal/file/file_plugin.go +++ b/internal/file/file_plugin.go @@ -31,12 +31,16 @@ type FilePlugin struct { config *config.Config conn grpc.GrpcConnectionInterface fileManagerService fileManagerServiceInterface + serverType model.ServerType } -func NewFilePlugin(agentConfig *config.Config, grpcConnection grpc.GrpcConnectionInterface) *FilePlugin { +func NewFilePlugin(agentConfig *config.Config, grpcConnection grpc.GrpcConnectionInterface, + serverType model.ServerType, +) *FilePlugin { return &FilePlugin{ - config: agentConfig, - conn: grpcConnection, + config: agentConfig, + conn: grpcConnection, + serverType: serverType, } } @@ -88,6 +92,15 @@ func (fp *FilePlugin) Process(ctx context.Context, msg *bus.Message) { } func (fp *FilePlugin) Subscriptions() []string { + if fp.serverType == model.Auxiliary { + return []string{ + bus.ConnectionResetTopic, + bus.ConnectionCreatedTopic, + bus.NginxConfigUpdateTopic, + bus.ConfigUploadRequestTopic, + } + } + return []string{ bus.ConnectionResetTopic, bus.ConnectionCreatedTopic, diff --git a/internal/file/file_plugin_test.go b/internal/file/file_plugin_test.go index c008d9de6..f1cb08403 100644 --- a/internal/file/file_plugin_test.go +++ b/internal/file/file_plugin_test.go @@ -31,7 +31,7 @@ import ( ) func TestFilePlugin_Info(t *testing.T) { - filePlugin := NewFilePlugin(types.AgentConfig(), &grpcfakes.FakeGrpcConnectionInterface{}) + filePlugin := NewFilePlugin(types.AgentConfig(), &grpcfakes.FakeGrpcConnectionInterface{}, model.Command) assert.Equal(t, "file", filePlugin.Info().Name) } @@ -39,14 +39,14 @@ func TestFilePlugin_Close(t *testing.T) { ctx := context.Background() fakeGrpcConnection := &grpcfakes.FakeGrpcConnectionInterface{} - filePlugin := NewFilePlugin(types.AgentConfig(), fakeGrpcConnection) + filePlugin := NewFilePlugin(types.AgentConfig(), fakeGrpcConnection, model.Command) filePlugin.Close(ctx) assert.Equal(t, 1, fakeGrpcConnection.CloseCallCount()) } func TestFilePlugin_Subscriptions(t *testing.T) { - filePlugin := NewFilePlugin(types.AgentConfig(), &grpcfakes.FakeGrpcConnectionInterface{}) + filePlugin := NewFilePlugin(types.AgentConfig(), &grpcfakes.FakeGrpcConnectionInterface{}, model.Command) assert.Equal( t, []string{ @@ -61,9 +61,16 @@ func TestFilePlugin_Subscriptions(t *testing.T) { }, filePlugin.Subscriptions(), ) + + readOnlyFilePlugin := NewFilePlugin(types.AgentConfig(), &grpcfakes.FakeGrpcConnectionInterface{}, model.Auxiliary) + assert.Equal(t, []string{ + bus.ConnectionResetTopic, + bus.ConnectionCreatedTopic, + bus.NginxConfigUpdateTopic, + bus.ConfigUploadRequestTopic, + }, readOnlyFilePlugin.Subscriptions()) } -// nolint: dupl func TestFilePlugin_Process_NginxConfigUpdateTopic(t *testing.T) { ctx := context.Background() @@ -86,7 +93,7 @@ func TestFilePlugin_Process_NginxConfigUpdateTopic(t *testing.T) { fakeGrpcConnection.FileServiceClientReturns(fakeFileServiceClient) messagePipe := busfakes.NewFakeMessagePipe() - filePlugin := NewFilePlugin(types.AgentConfig(), fakeGrpcConnection) + filePlugin := NewFilePlugin(types.AgentConfig(), fakeGrpcConnection, model.Command) err := filePlugin.Init(ctx, messagePipe) require.NoError(t, err) @@ -161,7 +168,7 @@ func TestFilePlugin_Process_ConfigApplyRequestTopic(t *testing.T) { fakeFileManagerService := &filefakes.FakeFileManagerServiceInterface{} fakeFileManagerService.ConfigApplyReturns(test.configApplyStatus, test.configApplyReturnsErr) messagePipe := busfakes.NewFakeMessagePipe() - filePlugin := NewFilePlugin(agentConfig, fakeGrpcConnection) + filePlugin := NewFilePlugin(agentConfig, fakeGrpcConnection, model.Command) err := filePlugin.Init(ctx, messagePipe) filePlugin.fileManagerService = fakeFileManagerService require.NoError(t, err) @@ -225,7 +232,6 @@ func TestFilePlugin_Process_ConfigApplyRequestTopic(t *testing.T) { } } -// nolint: dupl func TestFilePlugin_Process_ConfigUploadRequestTopic(t *testing.T) { ctx := context.Background() @@ -260,7 +266,7 @@ func TestFilePlugin_Process_ConfigUploadRequestTopic(t *testing.T) { fakeGrpcConnection.FileServiceClientReturns(fakeFileServiceClient) messagePipe := busfakes.NewFakeMessagePipe() - filePlugin := NewFilePlugin(types.AgentConfig(), fakeGrpcConnection) + filePlugin := NewFilePlugin(types.AgentConfig(), fakeGrpcConnection, model.Command) err := filePlugin.Init(ctx, messagePipe) require.NoError(t, err) @@ -315,7 +321,7 @@ func TestFilePlugin_Process_ConfigUploadRequestTopic_Failure(t *testing.T) { fakeGrpcConnection.FileServiceClientReturns(fakeFileServiceClient) messagePipe := busfakes.NewFakeMessagePipe() - filePlugin := NewFilePlugin(types.AgentConfig(), fakeGrpcConnection) + filePlugin := NewFilePlugin(types.AgentConfig(), fakeGrpcConnection, model.Command) err := filePlugin.Init(ctx, messagePipe) require.NoError(t, err) @@ -383,7 +389,7 @@ func TestFilePlugin_Process_ConfigApplyFailedTopic(t *testing.T) { messagePipe := busfakes.NewFakeMessagePipe() agentConfig := types.AgentConfig() - filePlugin := NewFilePlugin(agentConfig, fakeGrpcConnection) + filePlugin := NewFilePlugin(agentConfig, fakeGrpcConnection, model.Command) err := filePlugin.Init(ctx, messagePipe) require.NoError(t, err) @@ -430,7 +436,7 @@ func TestFilePlugin_Process_ConfigApplyRollbackCompleteTopic(t *testing.T) { messagePipe := busfakes.NewFakeMessagePipe() agentConfig := types.AgentConfig() fakeGrpcConnection := &grpcfakes.FakeGrpcConnectionInterface{} - filePlugin := NewFilePlugin(agentConfig, fakeGrpcConnection) + filePlugin := NewFilePlugin(agentConfig, fakeGrpcConnection, model.Command) err := filePlugin.Init(ctx, messagePipe) require.NoError(t, err) @@ -475,7 +481,7 @@ func TestFilePlugin_Process_ConfigApplyCompleteTopic(t *testing.T) { messagePipe := busfakes.NewFakeMessagePipe() agentConfig := types.AgentConfig() fakeGrpcConnection := &grpcfakes.FakeGrpcConnectionInterface{} - filePlugin := NewFilePlugin(agentConfig, fakeGrpcConnection) + filePlugin := NewFilePlugin(agentConfig, fakeGrpcConnection, model.Command) err := filePlugin.Init(ctx, messagePipe) require.NoError(t, err) diff --git a/internal/file/file_service_operator.go b/internal/file/file_service_operator.go index a3091c0c2..6bf835fa0 100644 --- a/internal/file/file_service_operator.go +++ b/internal/file/file_service_operator.go @@ -30,6 +30,8 @@ import ( "google.golang.org/protobuf/types/known/timestamppb" ) +// File service operator handles requests to the grpc file service + type FileServiceOperator struct { fileServiceClient mpi.FileServiceClient agentConfig *config.Config diff --git a/internal/file/read_only_plugin.go b/internal/file/read_only_plugin.go deleted file mode 100644 index 529be0293..000000000 --- a/internal/file/read_only_plugin.go +++ /dev/null @@ -1,158 +0,0 @@ -// Copyright (c) F5, Inc. -// -// This source code is licensed under the Apache License, Version 2.0 license found in the -// LICENSE file in the root directory of this source tree. - -package file - -import ( - "context" - "log/slog" - - "github.com/nginx/agent/v3/pkg/id" - - mpi "github.com/nginx/agent/v3/api/grpc/mpi/v1" - "github.com/nginx/agent/v3/internal/bus" - "github.com/nginx/agent/v3/internal/config" - "github.com/nginx/agent/v3/internal/grpc" - "github.com/nginx/agent/v3/internal/logger" - "github.com/nginx/agent/v3/internal/model" - "google.golang.org/protobuf/types/known/timestamppb" -) - -var _ bus.Plugin = (*ReadFilePlugin)(nil) - -// The file plugin only writes, deletes and checks hashes of files -// the file plugin does not care about the instance type - -type ReadFilePlugin struct { - messagePipe bus.MessagePipeInterface - config *config.Config - conn grpc.GrpcConnectionInterface - fileManagerService fileManagerServiceInterface -} - -func NewReadFilePlugin(agentConfig *config.Config, grpcConnection grpc.GrpcConnectionInterface) *ReadFilePlugin { - return &ReadFilePlugin{ - config: agentConfig, - conn: grpcConnection, - } -} - -func (rp *ReadFilePlugin) Init(ctx context.Context, messagePipe bus.MessagePipeInterface) error { - slog.DebugContext(ctx, "Starting read file plugin") - - rp.messagePipe = messagePipe - rp.fileManagerService = NewFileManagerService(rp.conn.FileServiceClient(), rp.config) - - return nil -} - -func (rp *ReadFilePlugin) Close(ctx context.Context) error { - slog.InfoContext(ctx, "Closing read file plugin") - return rp.conn.Close(ctx) -} - -func (rp *ReadFilePlugin) Info() *bus.Info { - return &bus.Info{ - Name: "read", - } -} - -func (rp *ReadFilePlugin) Process(ctx context.Context, msg *bus.Message) { - if logger.ServerType(ctx) == model.Auxiliary.String() || logger.ServerType(ctx) == "" { - switch msg.Topic { - case bus.ConnectionResetTopic: - rp.handleConnectionReset(ctx, msg) - case bus.ConnectionCreatedTopic: - slog.InfoContext(ctx, "Read file plugin received connection created message") - rp.fileManagerService.SetIsConnected(true) - case bus.NginxConfigUpdateTopic: - rp.handleNginxConfigUpdate(ctx, msg) - case bus.ConfigUploadRequestTopic: - rp.handleConfigUploadRequest(ctx, msg) - default: - slog.DebugContext(ctx, "Read file plugin received unknown topic", "topic", msg.Topic) - } - } -} - -func (rp *ReadFilePlugin) Subscriptions() []string { - return []string{ - bus.ConnectionResetTopic, - bus.ConnectionCreatedTopic, - bus.NginxConfigUpdateTopic, - bus.ConfigUploadRequestTopic, - } -} - -func (rp *ReadFilePlugin) handleConnectionReset(ctx context.Context, msg *bus.Message) { - slog.DebugContext(ctx, "Read file plugin received connection reset message") - if newConnection, ok := msg.Data.(grpc.GrpcConnectionInterface); ok { - var reconnect bool - err := rp.conn.Close(ctx) - if err != nil { - slog.ErrorContext(ctx, "Read file plugin unable to close connection", "error", err) - } - rp.conn = newConnection - - reconnect = rp.fileManagerService.IsConnected() - rp.fileManagerService = NewFileManagerService(rp.conn.FileServiceClient(), rp.config) - rp.fileManagerService.SetIsConnected(reconnect) - - slog.DebugContext(ctx, "File manager service client reset successfully") - } -} - -func (rp *ReadFilePlugin) handleNginxConfigUpdate(ctx context.Context, msg *bus.Message) { - slog.DebugContext(ctx, "Read file plugin received nginx config update message") - nginxConfigContext, ok := msg.Data.(*model.NginxConfigContext) - if !ok { - slog.ErrorContext(ctx, "Unable to cast message payload to *model.NginxConfigContext", "payload", msg.Data) - - return - } - - rp.fileManagerService.ConfigUpdate(ctx, nginxConfigContext) -} - -// nolint: dupl -func (rp *ReadFilePlugin) handleConfigUploadRequest(ctx context.Context, msg *bus.Message) { - slog.DebugContext(ctx, "Read file plugin received config upload request message") - managementPlaneRequest, ok := msg.Data.(*mpi.ManagementPlaneRequest) - if !ok { - slog.ErrorContext( - ctx, - "Unable to cast message payload to *mpi.ManagementPlaneRequest", - "payload", msg.Data, - ) - - return - } - - configUploadRequest := managementPlaneRequest.GetConfigUploadRequest() - - correlationID := logger.CorrelationID(ctx) - - updatingFilesError := rp.fileManagerService.ConfigUpload(ctx, configUploadRequest) - - response := &mpi.DataPlaneResponse{ - MessageMeta: &mpi.MessageMeta{ - MessageId: id.GenerateMessageID(), - CorrelationId: correlationID, - Timestamp: timestamppb.Now(), - }, - CommandResponse: &mpi.CommandResponse{ - Status: mpi.CommandResponse_COMMAND_STATUS_OK, - Message: "Successfully updated all files", - }, - } - - if updatingFilesError != nil { - response.CommandResponse.Status = mpi.CommandResponse_COMMAND_STATUS_FAILURE - response.CommandResponse.Message = "Failed to update all files" - response.CommandResponse.Error = updatingFilesError.Error() - } - - rp.messagePipe.Process(ctx, &bus.Message{Topic: bus.DataPlaneResponseTopic, Data: response}) -} diff --git a/internal/file/read_only_plugin_test.go b/internal/file/read_only_plugin_test.go deleted file mode 100644 index 5c6206d6a..000000000 --- a/internal/file/read_only_plugin_test.go +++ /dev/null @@ -1,247 +0,0 @@ -// Copyright (c) F5, Inc. -// -// This source code is licensed under the Apache License, Version 2.0 license found in the -// LICENSE file in the root directory of this source tree. - -package file - -import ( - "context" - "os" - "testing" - "time" - - mpi "github.com/nginx/agent/v3/api/grpc/mpi/v1" - "github.com/nginx/agent/v3/api/grpc/mpi/v1/v1fakes" - "github.com/nginx/agent/v3/internal/bus" - "github.com/nginx/agent/v3/internal/bus/busfakes" - "github.com/nginx/agent/v3/internal/grpc/grpcfakes" - "github.com/nginx/agent/v3/internal/model" - "github.com/nginx/agent/v3/test/helpers" - "github.com/nginx/agent/v3/test/protos" - "github.com/nginx/agent/v3/test/types" - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" -) - -// nolint: dupl -func TestReadOnlyPlugin_Process_NginxConfigUpdateTopic(t *testing.T) { - ctx := context.Background() - - fileMeta := protos.FileMeta("/etc/nginx/nginx/conf", "") - - message := &model.NginxConfigContext{ - Files: []*mpi.File{ - { - FileMeta: fileMeta, - }, - }, - } - - fakeFileServiceClient := &v1fakes.FakeFileServiceClient{} - fakeFileServiceClient.UpdateOverviewReturns(&mpi.UpdateOverviewResponse{ - Overview: nil, - }, nil) - - fakeGrpcConnection := &grpcfakes.FakeGrpcConnectionInterface{} - fakeGrpcConnection.FileServiceClientReturns(fakeFileServiceClient) - messagePipe := busfakes.NewFakeMessagePipe() - - readPlugin := NewReadFilePlugin(types.AgentConfig(), fakeGrpcConnection) - err := readPlugin.Init(ctx, messagePipe) - require.NoError(t, err) - - readPlugin.Process(ctx, &bus.Message{Topic: bus.ConnectionCreatedTopic}) - readPlugin.Process(ctx, &bus.Message{Topic: bus.NginxConfigUpdateTopic, Data: message}) - - assert.Eventually( - t, - func() bool { return fakeFileServiceClient.UpdateOverviewCallCount() == 1 }, - 2*time.Second, - 10*time.Millisecond, - ) -} - -func TestReadPlugin_ConnectionReset(t *testing.T) { - ctx := context.Background() - - fakeFileServiceClient := &v1fakes.FakeFileServiceClient{} - fakeGrpcConnection := &grpcfakes.FakeGrpcConnectionInterface{} - fakeGrpcConnection.FileServiceClientReturns(fakeFileServiceClient) - - newFakeFileServiceClient := &v1fakes.FakeFileServiceClient{} - newGrpcConnection := &grpcfakes.FakeGrpcConnectionInterface{} - newGrpcConnection.FileServiceClientReturns(newFakeFileServiceClient) - - messagePipe := busfakes.NewFakeMessagePipe() - - readPlugin := NewReadFilePlugin(types.AgentConfig(), fakeGrpcConnection) - err := readPlugin.Init(ctx, messagePipe) - require.NoError(t, err) - - message := &bus.Message{Topic: bus.ConnectionResetTopic, Data: newGrpcConnection} - - readPlugin.Process(ctx, message) - assert.Eventually(t, - func() bool { - return fakeGrpcConnection.CloseCallCount() == 1 - }, - 2*time.Second, - 10*time.Millisecond, - ) - - assert.Eventually(t, - func() bool { - return newGrpcConnection.FileServiceClientCallCount() == 1 - }, - 2*time.Second, - 10*time.Millisecond, - ) - - assert.Equal(t, newGrpcConnection, readPlugin.conn) -} - -// nolint: dupl -func TestReadPlugin_Process_ConfigUploadRequestTopic(t *testing.T) { - ctx := context.Background() - - tempDir := os.TempDir() - testFile := helpers.CreateFileWithErrorCheck(t, tempDir, "nginx.conf") - defer helpers.RemoveFileWithErrorCheck(t, testFile.Name()) - fileMeta := protos.FileMeta(testFile.Name(), "") - - message := &mpi.ManagementPlaneRequest{ - Request: &mpi.ManagementPlaneRequest_ConfigUploadRequest{ - ConfigUploadRequest: &mpi.ConfigUploadRequest{ - Overview: &mpi.FileOverview{ - Files: []*mpi.File{ - { - FileMeta: fileMeta, - }, - { - FileMeta: fileMeta, - }, - }, - ConfigVersion: &mpi.ConfigVersion{ - InstanceId: "123", - Version: "f33ref3d32d3c32d3a", - }, - }, - }, - }, - } - - fakeFileServiceClient := &v1fakes.FakeFileServiceClient{} - fakeGrpcConnection := &grpcfakes.FakeGrpcConnectionInterface{} - fakeGrpcConnection.FileServiceClientReturns(fakeFileServiceClient) - messagePipe := busfakes.NewFakeMessagePipe() - - readPlugin := NewReadFilePlugin(types.AgentConfig(), fakeGrpcConnection) - err := readPlugin.Init(ctx, messagePipe) - require.NoError(t, err) - - readPlugin.Process(ctx, &bus.Message{Topic: bus.ConnectionCreatedTopic}) - readPlugin.Process(ctx, &bus.Message{Topic: bus.ConfigUploadRequestTopic, Data: message}) - - assert.Eventually( - t, - func() bool { return fakeFileServiceClient.UpdateFileCallCount() == 2 }, - 2*time.Second, - 10*time.Millisecond, - ) - - messages := messagePipe.Messages() - assert.Len(t, messages, 1) - assert.Equal(t, bus.DataPlaneResponseTopic, messages[0].Topic) - - dataPlaneResponse, ok := messages[0].Data.(*mpi.DataPlaneResponse) - assert.True(t, ok) - assert.Equal( - t, - mpi.CommandResponse_COMMAND_STATUS_OK, - dataPlaneResponse.GetCommandResponse().GetStatus(), - ) -} - -func TestReadPlugin_Process_ConfigUploadRequestTopic_Failure(t *testing.T) { - ctx := context.Background() - - fileMeta := protos.FileMeta("/unknown/file.conf", "") - - message := &mpi.ManagementPlaneRequest{ - Request: &mpi.ManagementPlaneRequest_ConfigUploadRequest{ - ConfigUploadRequest: &mpi.ConfigUploadRequest{ - Overview: &mpi.FileOverview{ - Files: []*mpi.File{ - { - FileMeta: fileMeta, - }, - { - FileMeta: fileMeta, - }, - }, - ConfigVersion: protos.CreateConfigVersion(), - }, - }, - }, - } - - fakeFileServiceClient := &v1fakes.FakeFileServiceClient{} - fakeGrpcConnection := &grpcfakes.FakeGrpcConnectionInterface{} - fakeGrpcConnection.FileServiceClientReturns(fakeFileServiceClient) - messagePipe := busfakes.NewFakeMessagePipe() - - readPlugin := NewReadFilePlugin(types.AgentConfig(), fakeGrpcConnection) - err := readPlugin.Init(ctx, messagePipe) - require.NoError(t, err) - - readPlugin.Process(ctx, &bus.Message{Topic: bus.ConnectionCreatedTopic}) - readPlugin.Process(ctx, &bus.Message{Topic: bus.ConfigUploadRequestTopic, Data: message}) - - assert.Eventually( - t, - func() bool { return len(messagePipe.Messages()) == 1 }, - 2*time.Second, - 10*time.Millisecond, - ) - - assert.Equal(t, 0, fakeFileServiceClient.UpdateFileCallCount()) - - messages := messagePipe.Messages() - assert.Len(t, messages, 1) - - assert.Equal(t, bus.DataPlaneResponseTopic, messages[0].Topic) - - dataPlaneResponse, ok := messages[0].Data.(*mpi.DataPlaneResponse) - assert.True(t, ok) - assert.Equal( - t, - mpi.CommandResponse_COMMAND_STATUS_FAILURE, - dataPlaneResponse.GetCommandResponse().GetStatus(), - ) -} - -func TestReadPlugin_Subscriptions(t *testing.T) { - readPlugin := NewReadFilePlugin(types.AgentConfig(), &grpcfakes.FakeGrpcConnectionInterface{}) - assert.Equal(t, []string{ - bus.ConnectionResetTopic, - bus.ConnectionCreatedTopic, - bus.NginxConfigUpdateTopic, - bus.ConfigUploadRequestTopic, - }, readPlugin.Subscriptions()) -} - -func TestReadPlugin_Info(t *testing.T) { - filePlugin := NewReadFilePlugin(types.AgentConfig(), &grpcfakes.FakeGrpcConnectionInterface{}) - assert.Equal(t, "read", filePlugin.Info().Name) -} - -func TestReadPlugin_Close(t *testing.T) { - ctx := context.Background() - fakeGrpcConnection := &grpcfakes.FakeGrpcConnectionInterface{} - - filePlugin := NewReadFilePlugin(types.AgentConfig(), fakeGrpcConnection) - filePlugin.Close(ctx) - - assert.Equal(t, 1, fakeGrpcConnection.CloseCallCount()) -} diff --git a/internal/plugin/plugin_manager.go b/internal/plugin/plugin_manager.go index 2495afff6..30bcfc637 100644 --- a/internal/plugin/plugin_manager.go +++ b/internal/plugin/plugin_manager.go @@ -51,7 +51,7 @@ func addCommandAndFilePlugins(ctx context.Context, plugins []bus.Plugin, agentCo } else { commandPlugin := command.NewCommandPlugin(agentConfig, grpcConnection, model.Command) plugins = append(plugins, commandPlugin) - filePlugin := file.NewFilePlugin(agentConfig, grpcConnection) + filePlugin := file.NewFilePlugin(agentConfig, grpcConnection, model.Command) plugins = append(plugins, filePlugin) } } else { @@ -72,7 +72,7 @@ func addAuxiliaryCommandAndFilePlugins(ctx context.Context, plugins []bus.Plugin } else { auxCommandPlugin := command.NewCommandPlugin(agentConfig, auxGRPCConnection, model.Auxiliary) plugins = append(plugins, auxCommandPlugin) - readFilePlugin := file.NewReadFilePlugin(agentConfig, auxGRPCConnection) + readFilePlugin := file.NewFilePlugin(agentConfig, auxGRPCConnection, model.Auxiliary) plugins = append(plugins, readFilePlugin) } } else { diff --git a/internal/plugin/plugin_manager_test.go b/internal/plugin/plugin_manager_test.go index bf73936d1..53fe3663b 100644 --- a/internal/plugin/plugin_manager_test.go +++ b/internal/plugin/plugin_manager_test.go @@ -62,7 +62,7 @@ func TestLoadPlugins(t *testing.T) { &command.CommandPlugin{}, &file.FilePlugin{}, &command.CommandPlugin{}, - &file.ReadFilePlugin{}, + &file.FilePlugin{}, &watcher.Watcher{}, }, }, From 5773b421fb1c7fc3b3c4d55b9a38f0d42b86f620 Mon Sep 17 00:00:00 2001 From: Aphral Griffin Date: Mon, 23 Jun 2025 14:44:43 +0100 Subject: [PATCH 09/17] clean up --- internal/file/file_plugin.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/file/file_plugin.go b/internal/file/file_plugin.go index be99fffa6..c4c8ee6e8 100644 --- a/internal/file/file_plugin.go +++ b/internal/file/file_plugin.go @@ -66,7 +66,7 @@ func (fp *FilePlugin) Info() *bus.Info { // nolint: cyclop, revive func (fp *FilePlugin) Process(ctx context.Context, msg *bus.Message) { - if logger.ServerType(ctx) == model.Command.String() || logger.ServerType(ctx) == "" { + if logger.ServerType(ctx) == fp.serverType.String() || logger.ServerType(ctx) == "" { switch msg.Topic { case bus.ConnectionResetTopic: fp.handleConnectionReset(ctx, msg) From 8e26d8173278eec505089bca9d28f77c7c420033 Mon Sep 17 00:00:00 2001 From: Aphral Griffin Date: Fri, 27 Jun 2025 12:30:48 +0100 Subject: [PATCH 10/17] PR feedback --- internal/command/command_plugin.go | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/internal/command/command_plugin.go b/internal/command/command_plugin.go index d304934f3..abcc5d408 100644 --- a/internal/command/command_plugin.go +++ b/internal/command/command_plugin.go @@ -67,7 +67,7 @@ func (cp *CommandPlugin) Init(ctx context.Context, messagePipe bus.MessagePipeIn ctx, logger.ServerTypeContextKey, slog.Any(logger.ServerTypeKey, cp.commandServerType.String()), ) - slog.DebugContext(newCtx, "Starting command plugin", "command_server_type", cp.commandServerType.String()) + slog.DebugContext(newCtx, "Starting command plugin") cp.messagePipe = messagePipe cp.commandService = NewCommandService(cp.conn.CommandServiceClient(), cp.config, cp.subscribeChannel) @@ -78,7 +78,11 @@ func (cp *CommandPlugin) Init(ctx context.Context, messagePipe bus.MessagePipeIn } func (cp *CommandPlugin) Close(ctx context.Context) error { - slog.InfoContext(ctx, "Closing command plugin", "command_server_type", cp.commandServerType.String()) + newCtx := context.WithValue( + ctx, + logger.ServerTypeContextKey, slog.Any(logger.ServerTypeKey, cp.commandServerType.String()), + ) + slog.InfoContext(newCtx, "Closing command plugin") cp.subscribeMutex.Lock() if cp.subscribeCancel != nil { @@ -86,7 +90,7 @@ func (cp *CommandPlugin) Close(ctx context.Context) error { } cp.subscribeMutex.Unlock() - return cp.conn.Close(ctx) + return cp.conn.Close(newCtx) } func (cp *CommandPlugin) Info() *bus.Info { @@ -96,7 +100,7 @@ func (cp *CommandPlugin) Info() *bus.Info { } func (cp *CommandPlugin) Process(ctx context.Context, msg *bus.Message) { - slog.DebugContext(ctx, "Processing command", "command_server_type", logger.ServerType(ctx)) + slog.DebugContext(ctx, "Processing command") if logger.ServerType(ctx) == cp.commandServerType.String() || logger.ServerType(ctx) == "" { switch msg.Topic { @@ -183,8 +187,7 @@ func (cp *CommandPlugin) processInstanceHealth(ctx context.Context, msg *bus.Mes if instances, ok := msg.Data.([]*mpi.InstanceHealth); ok { err := cp.commandService.UpdateDataPlaneHealth(ctx, instances) if err != nil { - slog.ErrorContext(ctx, "Unable to update data plane health", "error", err, - "command_server_type", cp.commandServerType.String()) + slog.ErrorContext(ctx, "Unable to update data plane health", "error", err) } } } From f2d1fb2062cbd044f4df80f8a39e903a0e4ebf37 Mon Sep 17 00:00:00 2001 From: Aphral Griffin Date: Fri, 27 Jun 2025 14:01:55 +0100 Subject: [PATCH 11/17] PR feedback --- internal/command/command_plugin.go | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/internal/command/command_plugin.go b/internal/command/command_plugin.go index abcc5d408..233e425be 100644 --- a/internal/command/command_plugin.go +++ b/internal/command/command_plugin.go @@ -102,7 +102,14 @@ func (cp *CommandPlugin) Info() *bus.Info { func (cp *CommandPlugin) Process(ctx context.Context, msg *bus.Message) { slog.DebugContext(ctx, "Processing command") - if logger.ServerType(ctx) == cp.commandServerType.String() || logger.ServerType(ctx) == "" { + if logger.ServerType(ctx) == "" { + ctx = context.WithValue( + ctx, + logger.ServerTypeContextKey, slog.Any(logger.ServerTypeKey, cp.commandServerType.String()), + ) + } + + if logger.ServerType(ctx) == cp.commandServerType.String() { switch msg.Topic { case bus.ConnectionResetTopic: cp.processConnectionReset(ctx, msg) From 6d396696f838569426e40b09e20eb0424f62e30f Mon Sep 17 00:00:00 2001 From: Aphral Griffin Date: Fri, 27 Jun 2025 14:33:20 +0100 Subject: [PATCH 12/17] PR feedback --- internal/command/command_plugin.go | 7 ++++++- internal/file/file_plugin.go | 21 ++++++++++++++++++++- 2 files changed, 26 insertions(+), 2 deletions(-) diff --git a/internal/command/command_plugin.go b/internal/command/command_plugin.go index 233e425be..eaca8cb88 100644 --- a/internal/command/command_plugin.go +++ b/internal/command/command_plugin.go @@ -94,8 +94,13 @@ func (cp *CommandPlugin) Close(ctx context.Context) error { } func (cp *CommandPlugin) Info() *bus.Info { + name := "command" + if cp.commandServerType.String() == model.Auxiliary.String() { + name = "auxiliary-command" + } + return &bus.Info{ - Name: cp.commandServerType.String(), + Name: name, } } diff --git a/internal/file/file_plugin.go b/internal/file/file_plugin.go index c4c8ee6e8..bf0603b54 100644 --- a/internal/file/file_plugin.go +++ b/internal/file/file_plugin.go @@ -45,6 +45,10 @@ func NewFilePlugin(agentConfig *config.Config, grpcConnection grpc.GrpcConnectio } func (fp *FilePlugin) Init(ctx context.Context, messagePipe bus.MessagePipeInterface) error { + ctx = context.WithValue( + ctx, + logger.ServerTypeContextKey, slog.Any(logger.ServerTypeKey, fp.serverType.String()), + ) slog.DebugContext(ctx, "Starting file plugin") fp.messagePipe = messagePipe @@ -54,18 +58,33 @@ func (fp *FilePlugin) Init(ctx context.Context, messagePipe bus.MessagePipeInter } func (fp *FilePlugin) Close(ctx context.Context) error { + ctx = context.WithValue( + ctx, + logger.ServerTypeContextKey, slog.Any(logger.ServerTypeKey, fp.serverType.String()), + ) slog.InfoContext(ctx, "Closing file plugin") return fp.conn.Close(ctx) } func (fp *FilePlugin) Info() *bus.Info { + name := "file" + if fp.serverType.String() == model.Auxiliary.String() { + name = "auxiliary-file" + } return &bus.Info{ - Name: "file", + Name: name, } } // nolint: cyclop, revive func (fp *FilePlugin) Process(ctx context.Context, msg *bus.Message) { + if logger.ServerType(ctx) == "" { + ctx = context.WithValue( + ctx, + logger.ServerTypeContextKey, slog.Any(logger.ServerTypeKey, fp.serverType.String()), + ) + } + if logger.ServerType(ctx) == fp.serverType.String() || logger.ServerType(ctx) == "" { switch msg.Topic { case bus.ConnectionResetTopic: From 0d8652926ccd5816e0935499b8b0601fcb5d166e Mon Sep 17 00:00:00 2001 From: Aphral Griffin Date: Fri, 27 Jun 2025 14:36:02 +0100 Subject: [PATCH 13/17] PR feedback --- internal/file/file_plugin.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/internal/file/file_plugin.go b/internal/file/file_plugin.go index bf0603b54..c5c9d23ff 100644 --- a/internal/file/file_plugin.go +++ b/internal/file/file_plugin.go @@ -63,6 +63,7 @@ func (fp *FilePlugin) Close(ctx context.Context) error { logger.ServerTypeContextKey, slog.Any(logger.ServerTypeKey, fp.serverType.String()), ) slog.InfoContext(ctx, "Closing file plugin") + return fp.conn.Close(ctx) } @@ -71,6 +72,7 @@ func (fp *FilePlugin) Info() *bus.Info { if fp.serverType.String() == model.Auxiliary.String() { name = "auxiliary-file" } + return &bus.Info{ Name: name, } From 0a95a3859b1dd1ec2dd514c84eacc1604081d0f3 Mon Sep 17 00:00:00 2001 From: Aphral Griffin Date: Fri, 27 Jun 2025 14:40:24 +0100 Subject: [PATCH 14/17] PR feedback --- internal/command/command_plugin.go | 2 +- internal/file/file_plugin.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/command/command_plugin.go b/internal/command/command_plugin.go index eaca8cb88..30ed4769f 100644 --- a/internal/command/command_plugin.go +++ b/internal/command/command_plugin.go @@ -98,7 +98,7 @@ func (cp *CommandPlugin) Info() *bus.Info { if cp.commandServerType.String() == model.Auxiliary.String() { name = "auxiliary-command" } - + return &bus.Info{ Name: name, } diff --git a/internal/file/file_plugin.go b/internal/file/file_plugin.go index c5c9d23ff..6567830ea 100644 --- a/internal/file/file_plugin.go +++ b/internal/file/file_plugin.go @@ -72,7 +72,7 @@ func (fp *FilePlugin) Info() *bus.Info { if fp.serverType.String() == model.Auxiliary.String() { name = "auxiliary-file" } - + return &bus.Info{ Name: name, } From 3f39fbf5a9384555e962d53076b9939f833059eb Mon Sep 17 00:00:00 2001 From: Aphral Griffin Date: Fri, 27 Jun 2025 15:01:49 +0100 Subject: [PATCH 15/17] fix message response --- internal/command/command_plugin.go | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/internal/command/command_plugin.go b/internal/command/command_plugin.go index 233e425be..9e770f9c7 100644 --- a/internal/command/command_plugin.go +++ b/internal/command/command_plugin.go @@ -260,7 +260,7 @@ func (cp *CommandPlugin) monitorSubscribeChannel(ctx context.Context) { if cp.commandServerType != model.Command { slog.WarnContext(newCtx, "Auxiliary command server can not perform config apply", "command_server_type", cp.commandServerType.String()) - cp.handleInvalidRequest(newCtx, message) + cp.handleInvalidRequest(newCtx, message, "Config apply failed") return } @@ -273,7 +273,7 @@ func (cp *CommandPlugin) monitorSubscribeChannel(ctx context.Context) { if cp.commandServerType != model.Command { slog.WarnContext(newCtx, "Auxiliary command server can not perform api action", "command_server_type", cp.commandServerType.String()) - cp.handleInvalidRequest(newCtx, message) + cp.handleInvalidRequest(newCtx, message, "API action failed") return } @@ -365,15 +365,17 @@ func (cp *CommandPlugin) handleHealthRequest(newCtx context.Context) { cp.messagePipe.Process(newCtx, &bus.Message{Topic: bus.DataPlaneHealthRequestTopic}) } -func (cp *CommandPlugin) handleInvalidRequest(ctx context.Context, message *mpi.ManagementPlaneRequest) { +func (cp *CommandPlugin) handleInvalidRequest(ctx context.Context, + request *mpi.ManagementPlaneRequest, message string, +) { err := cp.commandService.SendDataPlaneResponse(ctx, &mpi.DataPlaneResponse{ - MessageMeta: message.GetMessageMeta(), + MessageMeta: request.GetMessageMeta(), CommandResponse: &mpi.CommandResponse{ Status: mpi.CommandResponse_COMMAND_STATUS_FAILURE, - Message: "Can not perform write action as auxiliary command server", - Error: "request not allowed", + Message: message, + Error: "Can not perform write action as auxiliary command server", }, - InstanceId: message.GetActionRequest().GetInstanceId(), + InstanceId: request.GetActionRequest().GetInstanceId(), }) if err != nil { slog.ErrorContext(ctx, "Unable to send data plane response", "error", err) From 44f1e1e52c8867ccf10e73fa1dfb52368f539f9e Mon Sep 17 00:00:00 2001 From: Aphral Griffin Date: Tue, 1 Jul 2025 16:43:55 +0100 Subject: [PATCH 16/17] merge main --- go.mod | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/go.mod b/go.mod index 434b15a13..e84730f75 100644 --- a/go.mod +++ b/go.mod @@ -66,6 +66,7 @@ require ( go.opentelemetry.io/collector/processor v1.30.0 go.opentelemetry.io/collector/processor/batchprocessor v0.124.0 go.opentelemetry.io/collector/processor/memorylimiterprocessor v0.124.0 + go.opentelemetry.io/collector/processor/processortest v0.124.0 go.opentelemetry.io/collector/receiver v1.30.0 go.opentelemetry.io/collector/receiver/otlpreceiver v0.124.0 go.opentelemetry.io/collector/receiver/receivertest v0.124.0 @@ -74,6 +75,7 @@ require ( go.opentelemetry.io/collector/scraper/scrapertest v0.124.0 go.opentelemetry.io/otel v1.35.0 go.uber.org/goleak v1.3.0 + go.uber.org/multierr v1.11.0 go.uber.org/zap v1.27.0 golang.org/x/mod v0.23.0 golang.org/x/sync v0.13.0 @@ -260,7 +262,6 @@ require ( go.opentelemetry.io/collector/pipeline/xpipeline v0.124.0 // indirect go.opentelemetry.io/collector/processor/processorhelper v0.124.0 // indirect go.opentelemetry.io/collector/processor/processorhelper/xprocessorhelper v0.124.0 // indirect - go.opentelemetry.io/collector/processor/processortest v0.124.0 // indirect go.opentelemetry.io/collector/processor/xprocessor v0.124.0 // indirect go.opentelemetry.io/collector/receiver/receiverhelper v0.124.0 // indirect go.opentelemetry.io/collector/receiver/xreceiver v0.124.0 // indirect @@ -289,7 +290,6 @@ require ( go.opentelemetry.io/otel/sdk/log v0.11.0 // indirect go.opentelemetry.io/otel/trace v1.35.0 // indirect go.opentelemetry.io/proto/otlp v1.5.0 // indirect - go.uber.org/multierr v1.11.0 // indirect golang.org/x/arch v0.12.0 // indirect golang.org/x/exp v0.0.0-20250210185358-939b2ce775ac // indirect golang.org/x/tools v0.30.0 // indirect From 845f3dd099c99e3cf0981f6ba57fb5812ce52038 Mon Sep 17 00:00:00 2001 From: Aphral Griffin Date: Tue, 1 Jul 2025 16:59:01 +0100 Subject: [PATCH 17/17] fix instance ID --- internal/command/command_plugin.go | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/internal/command/command_plugin.go b/internal/command/command_plugin.go index 9e770f9c7..995da5ac6 100644 --- a/internal/command/command_plugin.go +++ b/internal/command/command_plugin.go @@ -260,7 +260,8 @@ func (cp *CommandPlugin) monitorSubscribeChannel(ctx context.Context) { if cp.commandServerType != model.Command { slog.WarnContext(newCtx, "Auxiliary command server can not perform config apply", "command_server_type", cp.commandServerType.String()) - cp.handleInvalidRequest(newCtx, message, "Config apply failed") + cp.handleInvalidRequest(newCtx, message, "Config apply failed", + message.GetConfigApplyRequest().GetOverview().GetConfigVersion().GetInstanceId()) return } @@ -273,7 +274,8 @@ func (cp *CommandPlugin) monitorSubscribeChannel(ctx context.Context) { if cp.commandServerType != model.Command { slog.WarnContext(newCtx, "Auxiliary command server can not perform api action", "command_server_type", cp.commandServerType.String()) - cp.handleInvalidRequest(newCtx, message, "API action failed") + cp.handleInvalidRequest(newCtx, message, "API action failed", + message.GetActionRequest().GetInstanceId()) return } @@ -366,7 +368,7 @@ func (cp *CommandPlugin) handleHealthRequest(newCtx context.Context) { } func (cp *CommandPlugin) handleInvalidRequest(ctx context.Context, - request *mpi.ManagementPlaneRequest, message string, + request *mpi.ManagementPlaneRequest, message, instanceID string, ) { err := cp.commandService.SendDataPlaneResponse(ctx, &mpi.DataPlaneResponse{ MessageMeta: request.GetMessageMeta(), @@ -375,7 +377,7 @@ func (cp *CommandPlugin) handleInvalidRequest(ctx context.Context, Message: message, Error: "Can not perform write action as auxiliary command server", }, - InstanceId: request.GetActionRequest().GetInstanceId(), + InstanceId: instanceID, }) if err != nil { slog.ErrorContext(ctx, "Unable to send data plane response", "error", err)