From 8fa1312b64d6a9156b17a4a9a091e004c5dea0d1 Mon Sep 17 00:00:00 2001 From: Aphral Griffin Date: Tue, 15 Apr 2025 16:44:28 +0100 Subject: [PATCH 1/4] enable default collector --- internal/config/config.go | 60 ++++++++++++++++++++++++ scripts/packages/preinstall.sh | 26 ---------- scripts/packages/upgrade-agent-config.sh | 26 ---------- 3 files changed, 60 insertions(+), 52 deletions(-) diff --git a/internal/config/config.go b/internal/config/config.go index 0861c2f7f..203884490 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -16,6 +16,7 @@ import ( "slices" "strconv" "strings" + "time" uuidLibrary "github.com/nginx/agent/v3/pkg/id" selfsignedcerts "github.com/nginx/agent/v3/pkg/tls" @@ -115,6 +116,8 @@ func ResolveConfig() (*Config, error) { Labels: resolveLabels(), } + checkCollectorConfiguration(collector, config) + slog.Debug("Agent config", "config", config) slog.Info("Enabled features", "features", config.Features) slog.Info("Excluded files from being watched for file changes", "exclude_files", @@ -123,6 +126,63 @@ func ResolveConfig() (*Config, error) { return config, nil } +func checkCollectorConfiguration(collector *Collector, config *Config) { + if len(collector.Exporters.OtlpExporters) == 0 && collector.Exporters.PrometheusExporter == nil && + collector.Exporters.Debug == nil && isGrpcClientConfigured(config) { + slog.Info("No collector configuration found in NGINX Agent config, command server configuration found." + + "Using default collector configuration") + defaultCollector(collector, config) + } +} + +func defaultCollector(collector *Collector, config *Config) { + token := config.Command.Auth.Token + if config.Command.Auth.TokenPath != "" { + token = config.Command.Auth.TokenPath + } + + collector.Receivers.HostMetrics = &HostMetrics{ + Scrapers: &HostMetricsScrapers{ + CPU: &CPUScraper{}, + Disk: &DiskScraper{}, + Filesystem: &FilesystemScraper{}, + Memory: &MemoryScraper{}, + Network: nil, + }, + CollectionInterval: 1 * time.Minute, + InitialDelay: 1 * time.Second, + } + + collector.Exporters.OtlpExporters = append(collector.Exporters.OtlpExporters, OtlpExporter{ + Server: config.Command.Server, + TLS: config.Command.TLS, + Compression: "", + Authenticator: "headers_setter", + }) + + header := []Header{ + { + Action: "insert", + Key: "authorization", + Value: token, + }, + } + collector.Extensions.HeadersSetter = &HeadersSetter{ + Headers: header, + } +} + +func isGrpcClientConfigured(agentConfig *Config) bool { + return agentConfig.Command != nil && + agentConfig.Command.Server != nil && + agentConfig.Command.Server.Host != "" && + agentConfig.Command.Server.Port != 0 && + agentConfig.Command.Server.Type == Grpc && + agentConfig.Command.Auth != nil && + (agentConfig.Command.Auth.Token != "" || agentConfig.Command.Auth.TokenPath != "") && + agentConfig.Command.TLS != nil +} + func setVersion(version, commit string) { RootCommand.Version = version + "-" + commit viperInstance.SetDefault(VersionKey, version) diff --git a/scripts/packages/preinstall.sh b/scripts/packages/preinstall.sh index 88b477c94..d0a105901 100644 --- a/scripts/packages/preinstall.sh +++ b/scripts/packages/preinstall.sh @@ -114,32 +114,6 @@ command: token: ${token} tls: skip_verify: false - -collector: - receivers: - host_metrics: - scrapers: - cpu: {} - memory: {} - disk: {} - network: {} - filesystem: {} - processors: - batch: {} - exporters: - otlp_exporters: - - server: - host: ${NGINX_ONE_HOST} - port: 443 - authenticator: headers_setter - tls: - skip_verify: false - extensions: - headers_setter: - headers: - - action: insert - key: \"authorization\" - value: ${token} " echo "${v3_config_contents}" > $v3_config_file diff --git a/scripts/packages/upgrade-agent-config.sh b/scripts/packages/upgrade-agent-config.sh index 8a417c849..0a3fefe9f 100755 --- a/scripts/packages/upgrade-agent-config.sh +++ b/scripts/packages/upgrade-agent-config.sh @@ -76,32 +76,6 @@ command: token: ${token} tls: skip_verify: false - -collector: - receivers: - host_metrics: - scrapers: - cpu: {} - memory: {} - disk: {} - network: {} - filesystem: {} - processors: - batch: {} - exporters: - otlp_exporters: - - server: - host: ${NGINX_ONE_HOST} - port: 443 - authenticator: headers_setter - tls: - skip_verify: false - extensions: - headers_setter: - headers: - - action: insert - key: \"authorization\" - value: ${token} " echo "${v3_config_contents}" > $v3_config_file From ab2869ac8bd353e350f64d859bf9ee14b353430e Mon Sep 17 00:00:00 2001 From: Aphral Griffin Date: Fri, 18 Apr 2025 15:00:46 +0100 Subject: [PATCH 2/4] fix token path for default --- internal/config/config.go | 25 +++++------ internal/config/types.go | 17 +++++++ internal/datasource/file/file.go | 37 ++++++++++++++++ internal/datasource/file/file_test.go | 64 +++++++++++++++++++++++++++ internal/grpc/grpc.go | 29 ++---------- internal/grpc/grpc_test.go | 61 ------------------------- internal/plugin/plugin_manager.go | 10 +---- 7 files changed, 134 insertions(+), 109 deletions(-) create mode 100644 internal/datasource/file/file.go create mode 100644 internal/datasource/file/file_test.go diff --git a/internal/config/config.go b/internal/config/config.go index 203884490..5cbc81ee7 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -18,6 +18,8 @@ import ( "strings" "time" + "github.com/nginx/agent/v3/internal/datasource/file" + uuidLibrary "github.com/nginx/agent/v3/pkg/id" selfsignedcerts "github.com/nginx/agent/v3/pkg/tls" "github.com/spf13/cobra" @@ -128,7 +130,8 @@ func ResolveConfig() (*Config, error) { func checkCollectorConfiguration(collector *Collector, config *Config) { if len(collector.Exporters.OtlpExporters) == 0 && collector.Exporters.PrometheusExporter == nil && - collector.Exporters.Debug == nil && isGrpcClientConfigured(config) { + collector.Exporters.Debug == nil && config.IsGrpcClientConfigured() && config.IsAuthConfigured() && + config.IsTLSConfigured() { slog.Info("No collector configuration found in NGINX Agent config, command server configuration found." + "Using default collector configuration") defaultCollector(collector, config) @@ -138,7 +141,14 @@ func checkCollectorConfiguration(collector *Collector, config *Config) { func defaultCollector(collector *Collector, config *Config) { token := config.Command.Auth.Token if config.Command.Auth.TokenPath != "" { - token = config.Command.Auth.TokenPath + pathToken, err := file.RetrieveTokenFromFile(config.Command.Auth.TokenPath) + if err != nil { + slog.Error("Error adding token to default collector, "+ + "default collector configuration not started", "error", err) + + return + } + token = pathToken } collector.Receivers.HostMetrics = &HostMetrics{ @@ -172,17 +182,6 @@ func defaultCollector(collector *Collector, config *Config) { } } -func isGrpcClientConfigured(agentConfig *Config) bool { - return agentConfig.Command != nil && - agentConfig.Command.Server != nil && - agentConfig.Command.Server.Host != "" && - agentConfig.Command.Server.Port != 0 && - agentConfig.Command.Server.Type == Grpc && - agentConfig.Command.Auth != nil && - (agentConfig.Command.Auth.Token != "" || agentConfig.Command.Auth.TokenPath != "") && - agentConfig.Command.TLS != nil -} - func setVersion(version, commit string) { RootCommand.Version = version + "-" + commit viperInstance.SetDefault(VersionKey, version) diff --git a/internal/config/types.go b/internal/config/types.go index 3afb29756..ad7b95b69 100644 --- a/internal/config/types.go +++ b/internal/config/types.go @@ -344,6 +344,23 @@ func (c *Config) IsDirectoryAllowed(directory string) bool { return isAllowedDir(directory, c.AllowedDirectories) } +func (c *Config) IsGrpcClientConfigured() bool { + return c.Command != nil && + c.Command.Server != nil && + c.Command.Server.Host != "" && + c.Command.Server.Port != 0 && + c.Command.Server.Type == Grpc +} + +func (c *Config) IsAuthConfigured() bool { + return c.Command.Auth != nil && + (c.Command.Auth.Token != "" || c.Command.Auth.TokenPath != "") +} + +func (c *Config) IsTLSConfigured() bool { + return c.Command.TLS != nil +} + func (c *Config) IsFeatureEnabled(feature string) bool { for _, enabledFeature := range c.Features { if enabledFeature == feature { diff --git a/internal/datasource/file/file.go b/internal/datasource/file/file.go new file mode 100644 index 000000000..59117898b --- /dev/null +++ b/internal/datasource/file/file.go @@ -0,0 +1,37 @@ +// 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 ( + "bytes" + "errors" + "fmt" + "log/slog" + "os" +) + +func RetrieveTokenFromFile(path string) (string, error) { + if path == "" { + return "", errors.New("token file path is empty") + } + + slog.Debug("Reading token from file", "path", path) + var keyVal string + keyBytes, err := os.ReadFile(path) + if err != nil { + return "", fmt.Errorf("unable to read token from file: %w", err) + } + + keyBytes = bytes.TrimSpace(keyBytes) + keyBytes = bytes.TrimRight(keyBytes, "\n") + keyVal = string(keyBytes) + + if keyVal == "" { + return "", errors.New("failed to load token, token file is empty") + } + + return keyVal, nil +} diff --git a/internal/datasource/file/file_test.go b/internal/datasource/file/file_test.go new file mode 100644 index 000000000..73a7d1e6c --- /dev/null +++ b/internal/datasource/file/file_test.go @@ -0,0 +1,64 @@ +// 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 ( + "os" + "testing" + + "github.com/nginx/agent/v3/test/helpers" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func Test_RetrieveTokenFromFile(t *testing.T) { + dir := t.TempDir() + tokenFile := helpers.CreateFileWithErrorCheck(t, dir, "test-tkn") + defer helpers.RemoveFileWithErrorCheck(t, tokenFile.Name()) + tests := []struct { + name string + path string + expected string + expectedErrMsg string + createToken bool + }{ + { + name: "Test 1: File exists", + createToken: true, + path: tokenFile.Name(), + expected: "test-tkn", + expectedErrMsg: "", + }, + { + name: "Test 2: File does not exist", + createToken: false, + path: "test-tkn", + expected: "", + expectedErrMsg: "unable to read token from file: open test-tkn: no such file or directory", + }, + { + name: "Test 3: Empty path", + createToken: false, + path: "", + expected: "", + expectedErrMsg: "token file path is empty", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if tt.createToken { + writeErr := os.WriteFile(tokenFile.Name(), []byte(" test-tkn\n"), 0o600) + require.NoError(t, writeErr) + } + + token, err := RetrieveTokenFromFile(tt.path) + if err != nil { + assert.Equal(t, tt.expectedErrMsg, err.Error()) + } + assert.Equal(t, tt.expected, token) + }) + } +} diff --git a/internal/grpc/grpc.go b/internal/grpc/grpc.go index 3a7a2ca09..78fa212be 100644 --- a/internal/grpc/grpc.go +++ b/internal/grpc/grpc.go @@ -6,17 +6,17 @@ package grpc import ( - "bytes" "context" "crypto/tls" "errors" "fmt" "log/slog" "net" - "os" "strings" "sync" + "github.com/nginx/agent/v3/internal/datasource/file" + "github.com/cenkalti/backoff/v4" grpcRetry "github.com/grpc-ecosystem/go-grpc-middleware/retry" @@ -243,7 +243,7 @@ func addPerRPCCredentials(agentConfig *config.Config, resourceID string, opts [] token := agentConfig.Command.Auth.Token if agentConfig.Command.Auth.TokenPath != "" { - tk, err := retrieveTokenFromFile(agentConfig.Command.Auth.TokenPath) + tk, err := file.RetrieveTokenFromFile(agentConfig.Command.Auth.TokenPath) if err == nil { token = tk } else { @@ -263,29 +263,6 @@ func addPerRPCCredentials(agentConfig *config.Config, resourceID string, opts [] return opts } -func retrieveTokenFromFile(path string) (string, error) { - if path == "" { - return "", errors.New("token file path is empty") - } - - slog.Debug("Reading token from file", "path", path) - var keyVal string - keyBytes, err := os.ReadFile(path) - if err != nil { - return "", fmt.Errorf("unable to read token from file: %w", err) - } - - keyBytes = bytes.TrimSpace(keyBytes) - keyBytes = bytes.TrimRight(keyBytes, "\n") - keyVal = string(keyBytes) - - if keyVal == "" { - return "", errors.New("failed to load token, token file is empty") - } - - return keyVal, nil -} - // Have to create our own UnaryClientInterceptor function since protovalidate only provides a UnaryServerInterceptor // https://pkg.go.dev/github.com/grpc-ecosystem/go-grpc-middleware/v2@v2.1.0/interceptors/protovalidate func ProtoValidatorUnaryClientInterceptor() (grpc.UnaryClientInterceptor, error) { diff --git a/internal/grpc/grpc_test.go b/internal/grpc/grpc_test.go index 989e6fdd9..4aea9ed6b 100644 --- a/internal/grpc/grpc_test.go +++ b/internal/grpc/grpc_test.go @@ -8,7 +8,6 @@ package grpc import ( "context" "fmt" - "os" "testing" "google.golang.org/grpc/credentials" @@ -356,66 +355,6 @@ func Test_ValidateGrpcError(t *testing.T) { assert.IsType(t, &backoff.PermanentError{}, result) } -// nolint:revive,gocognit -func Test_retrieveTokenFromFile(t *testing.T) { - tests := []struct { - name string - path string - want string - wantErrMsg string - createToken bool - }{ - { - name: "Test 1: File exists", - createToken: true, - path: "test-tkn", - want: "test-tkn", - wantErrMsg: "", - }, - { - name: "Test 2: File does not exist", - createToken: false, - path: "test-tkn", - want: "", - wantErrMsg: "unable to read token from file: open test-tkn: no such file or directory", - }, - { - name: "Test 3: Empty path", - createToken: false, - path: "", - want: "", - wantErrMsg: "token file path is empty", - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - defer func() { - if tt.createToken { - err := os.Remove(tt.path) - if err != nil { - t.Log(err) - } - } - }() - - if tt.createToken { - err := os.WriteFile(tt.path, []byte(tt.path), 0o600) - if err != nil { - t.Fatal(err) - } - } - - got, err := retrieveTokenFromFile(tt.path) - if err != nil { - if err.Error() != tt.wantErrMsg { - t.Errorf("retrieveTokenFromFile() error = %v, wantErr %v", err, tt.wantErrMsg) - } - } - assert.Equalf(t, tt.want, got, "retrieveTokenFromFile(%v)", tt.path) - }) - } -} - func Test_getTransportCredentials(t *testing.T) { tests := []struct { want credentials.TransportCredentials diff --git a/internal/plugin/plugin_manager.go b/internal/plugin/plugin_manager.go index 7744ce783..df1a0894d 100644 --- a/internal/plugin/plugin_manager.go +++ b/internal/plugin/plugin_manager.go @@ -39,7 +39,7 @@ func addResourcePlugin(plugins []bus.Plugin, agentConfig *config.Config) []bus.P } func addCommandAndFilePlugins(ctx context.Context, plugins []bus.Plugin, agentConfig *config.Config) []bus.Plugin { - if isGrpcClientConfigured(agentConfig) { + if agentConfig.IsGrpcClientConfigured() { grpcConnection, err := grpc.NewGrpcConnection(ctx, agentConfig) if err != nil { slog.WarnContext(ctx, "Failed to create gRPC connection", "error", err) @@ -79,11 +79,3 @@ func addWatcherPlugin(plugins []bus.Plugin, agentConfig *config.Config) []bus.Pl return plugins } - -func isGrpcClientConfigured(agentConfig *config.Config) bool { - return agentConfig.Command != nil && - agentConfig.Command.Server != nil && - agentConfig.Command.Server.Host != "" && - agentConfig.Command.Server.Port != 0 && - agentConfig.Command.Server.Type == config.Grpc -} From 2d63f6cf37124941cf1d972c7601c9b0b4888fcb Mon Sep 17 00:00:00 2001 From: Aphral Griffin Date: Tue, 22 Apr 2025 14:34:01 +0100 Subject: [PATCH 3/4] PR feedback --- internal/config/config.go | 11 ++++++++--- internal/datasource/file/file.go | 26 +++++++++++++------------- internal/datasource/file/file_test.go | 4 ++-- internal/grpc/grpc.go | 3 ++- 4 files changed, 25 insertions(+), 19 deletions(-) diff --git a/internal/config/config.go b/internal/config/config.go index 5cbc81ee7..d5eaf872c 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -129,8 +129,7 @@ func ResolveConfig() (*Config, error) { } func checkCollectorConfiguration(collector *Collector, config *Config) { - if len(collector.Exporters.OtlpExporters) == 0 && collector.Exporters.PrometheusExporter == nil && - collector.Exporters.Debug == nil && config.IsGrpcClientConfigured() && config.IsAuthConfigured() && + if isOTelExporterConfigured(collector) && config.IsGrpcClientConfigured() && config.IsAuthConfigured() && config.IsTLSConfigured() { slog.Info("No collector configuration found in NGINX Agent config, command server configuration found." + "Using default collector configuration") @@ -141,7 +140,8 @@ func checkCollectorConfiguration(collector *Collector, config *Config) { func defaultCollector(collector *Collector, config *Config) { token := config.Command.Auth.Token if config.Command.Auth.TokenPath != "" { - pathToken, err := file.RetrieveTokenFromFile(config.Command.Auth.TokenPath) + slog.Debug("Reading token from file", "path", config.Command.Auth.TokenPath) + pathToken, err := file.ReadFromFile(config.Command.Auth.TokenPath) if err != nil { slog.Error("Error adding token to default collector, "+ "default collector configuration not started", "error", err) @@ -970,3 +970,8 @@ func resolveMapStructure(key string, object any) error { return nil } + +func isOTelExporterConfigured(collector *Collector) bool { + return len(collector.Exporters.OtlpExporters) == 0 && collector.Exporters.PrometheusExporter == nil && + collector.Exporters.Debug == nil +} diff --git a/internal/datasource/file/file.go b/internal/datasource/file/file.go index 59117898b..d69472ab9 100644 --- a/internal/datasource/file/file.go +++ b/internal/datasource/file/file.go @@ -9,29 +9,29 @@ import ( "bytes" "errors" "fmt" - "log/slog" "os" ) -func RetrieveTokenFromFile(path string) (string, error) { +// ReadFromFile reads the contents from a file, trims the white space, trims newlines +// then returns the contents as a string +func ReadFromFile(path string) (string, error) { if path == "" { - return "", errors.New("token file path is empty") + return "", errors.New("failed to read file since file path is empty") } - slog.Debug("Reading token from file", "path", path) - var keyVal string - keyBytes, err := os.ReadFile(path) + var content string + contentBytes, err := os.ReadFile(path) if err != nil { - return "", fmt.Errorf("unable to read token from file: %w", err) + return "", fmt.Errorf("unable to read from file: %w", err) } - keyBytes = bytes.TrimSpace(keyBytes) - keyBytes = bytes.TrimRight(keyBytes, "\n") - keyVal = string(keyBytes) + contentBytes = bytes.TrimSpace(contentBytes) + contentBytes = bytes.TrimRight(contentBytes, "\n") + content = string(contentBytes) - if keyVal == "" { - return "", errors.New("failed to load token, token file is empty") + if content == "" { + return "", errors.New("failed to read from file, file is empty") } - return keyVal, nil + return content, nil } diff --git a/internal/datasource/file/file_test.go b/internal/datasource/file/file_test.go index 73a7d1e6c..6d66fd53b 100644 --- a/internal/datasource/file/file_test.go +++ b/internal/datasource/file/file_test.go @@ -44,7 +44,7 @@ func Test_RetrieveTokenFromFile(t *testing.T) { createToken: false, path: "", expected: "", - expectedErrMsg: "token file path is empty", + expectedErrMsg: "failed to read file since file path is empty", }, } for _, tt := range tests { @@ -54,7 +54,7 @@ func Test_RetrieveTokenFromFile(t *testing.T) { require.NoError(t, writeErr) } - token, err := RetrieveTokenFromFile(tt.path) + token, err := ReadFromFile(tt.path) if err != nil { assert.Equal(t, tt.expectedErrMsg, err.Error()) } diff --git a/internal/grpc/grpc.go b/internal/grpc/grpc.go index 78fa212be..5a98a8919 100644 --- a/internal/grpc/grpc.go +++ b/internal/grpc/grpc.go @@ -243,7 +243,8 @@ func addPerRPCCredentials(agentConfig *config.Config, resourceID string, opts [] token := agentConfig.Command.Auth.Token if agentConfig.Command.Auth.TokenPath != "" { - tk, err := file.RetrieveTokenFromFile(agentConfig.Command.Auth.TokenPath) + slog.Debug("Reading token from file", "path", agentConfig.Command.Auth.TokenPath) + tk, err := file.ReadFromFile(agentConfig.Command.Auth.TokenPath) if err == nil { token = tk } else { From c07604c7ab77f5854a2b2f4b8eb5f51be37dd024 Mon Sep 17 00:00:00 2001 From: Aphral Griffin Date: Tue, 22 Apr 2025 14:44:59 +0100 Subject: [PATCH 4/4] unit test --- internal/datasource/file/file_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/datasource/file/file_test.go b/internal/datasource/file/file_test.go index 6d66fd53b..c0727b5c1 100644 --- a/internal/datasource/file/file_test.go +++ b/internal/datasource/file/file_test.go @@ -37,7 +37,7 @@ func Test_RetrieveTokenFromFile(t *testing.T) { createToken: false, path: "test-tkn", expected: "", - expectedErrMsg: "unable to read token from file: open test-tkn: no such file or directory", + expectedErrMsg: "unable to read from file: open test-tkn: no such file or directory", }, { name: "Test 3: Empty path",