From 62bbfb8f438b9345e9038bba404b71986f309e82 Mon Sep 17 00:00:00 2001 From: oliveromahony Date: Fri, 27 Sep 2024 15:11:39 +0100 Subject: [PATCH] Added default set of features to Agent Configuration (#868) * added initial feature set and documentation --- docs/features/features.md | 211 ++++++++++++++++++ internal/config/config.go | 7 + internal/config/defaults.go | 11 + internal/config/flags.go | 1 + internal/config/types.go | 1 + .../instance/instance_watcher_service.go | 2 +- nginx-agent.conf | 34 +-- pkg/config/features.go | 18 ++ 8 files changed, 268 insertions(+), 17 deletions(-) create mode 100644 docs/features/features.md create mode 100644 pkg/config/features.go diff --git a/docs/features/features.md b/docs/features/features.md new file mode 100644 index 0000000000..f63763adc8 --- /dev/null +++ b/docs/features/features.md @@ -0,0 +1,211 @@ +# NGINX Agent Features + +- [Introduction](#introduction) +- [Feature Flags Overview](#feature-flags-overview) +- [Conflicting Combinations](#conflicting-combinations) +- [Cobra CLI Parameters](#cobra-cli-parameters) +- [Environment Variables](#environment-variables) +- [Configuration File](#configuration-file) +- [Dynamic Updates via gRPC](#dynamic-updates-via-grpc) +- [Internal State Management](#internal-state-management) +- [Code Path Management](#code-path-management) +- [Dynamic Feature Toggling](#dynamic-feature-toggling) +- [Security Considerations](#security-considerations) +- [Conclusion](#conclusion) + +## Introduction + +This document outlines the design of a feature flag management system +for the NGINX Agent. The system enables dynamic toggling of features +based on configurations provided via multiple sources, including CLI +parameters, environment variables, configuration files, and via protobuf +gRPC messages. Feature flags control the execution paths within the +application, allowing for flexible and controlled feature management. + +## Feature Flags Overview + +The design goal of this document is to capture fine grained features in +NGINX Agent v3. + +The ultimate goal of this design is to delegate fine-grained control of +high-level feature sets to the configuration. + +The system manages the following feature flags: + +| Feature Flag | Sub Category 1 | Description | Default | +|---------------|--------------------|--------------------------------------------------------------------------------------------------------------------------------------------------|-----------------------------------------------| +| configuration | | Full read/write management of configurations toggled by DataPlaneConfig ConfigMode | On | +| certificates | | Inclusion of public keys and other certificates in the configurations toggled by DataPlaneConfig CertMode | Off | +| connection | | Sends an initial connection message reporting instance information on presence of Command ServerConfig Host and Port | On | +| file-watcher | | Monitoring of file changes in allowed directories; may trigger DataPlane (DP) => ManagementPlane synchronisation of files configured by Watchers | On | +| agent-api | | REST Application Programming Interface (API) for NGINX Agent | Off | +| metrics | | Full metrics reporting | On | +| | metrics-host | All host-level metrics | On (if running in a host, virtualised or not) | +| | metrics-container | Container-level metrics read from cgroup information | On (if running in a container) | +| | metrics-instance | All OSS and Plus Metrics, depending on what instance is present | On (if instance present e.g. NGINX) | + +## Conflicting Combinations + +If there is a feature flag enabled and conflicting features are +enabled, the more specific feature flag takes precedence. If the +feature flag is set e.g. metrics-container in a non-containerised +execution environment, then no useful metrics will be reported. + +**config-certificates** being read-only and reporting ssl under +instances may conflict. + +**metrics** will include every specified metrics category (hence the +inherited specification). If fine-grained metrics are required, this +needs to be absent from the list. + +### Cobra CLI Parameters + +**Usage**: Users can enable or disable features via CLI flags when + launching the application. + +**Example**: + +```bash +./nginx-agent --features=connection,config,metrics,process-watcher,file-watcher,agent-api +``` + +Specifies a comma-separated list of features enabled for the Agent. + +- **Implementation**: Utilises [Cobra](https://github.com/spf13/cobra) + to define and parse CLI flags for each feature. + +### Environment Variables + +- **Usage**: Environment variables provide an alternative + configuration method, suitable for containerised deployments. + +- **Naming Convention**: NGINX_AGENT_FEATURES and is added to the list + +- **Implementation**: Use a library + [Viper](https://github.com/spf13/viper) to bind environment + variables to feature flags. + +### Configuration File + +- **Usage**: A configuration file (e.g., YAML, JSON, TOML) can list + enabled features and their parameters. + +- **Example (YAML)**: + +```yaml +features: +- connection +- configuration +- metrics +- process-watcher +- file-watcher +- agent-api + +``` + +**Implementation**: Parse the configuration file during initialisation using Viper. + +### Dynamic Updates via gRPC + +- Through the MPI + send an updated AgentConfig message with a different list of + features. + +## Internal State Management + +- **Singleton Pattern**: Implement a singleton FeaturePlugin to + maintain the state of all feature flags. On configuration change, + use the message bus to notify throughout the application of any + changes. + +- **Thread-Safety**: Use synchronisation mechanisms to ensure safe + concurrent access of changes + +## Code Path Management + +**Conditional Execution**: Use the FeatureManager to check feature + states before executing specific code paths. + +**Example**: + +```go + if featureManager.IsEnabled("metrics") { + // Execute full metrics reporting code path + } else { + // Execute alternative or no-op code path + } +``` + +- **Abstraction**: Encapsulate feature checks within helper functions + or middleware to streamline conditional logic across the codebase. + +## Dynamic Feature Toggling + +Implement methods within FeaturePlugin to enable, disable, and retrieve +feature states. Watch the nginx-agent.conf file for changes. Listen to +gRPC messages for AgentConfig changes. + +Example useful functionality: + +```go + +func (fm *FeatureManager) IsEnabled(featureName string) bool { + + fm.mutex.RLock() + + defer fm.mutex.RUnlock() + + // Code check to see if the feature is enabled in the AgentConfig + +} + +func (fm *FeatureManager) UpdateFeature(featureName string, enabled bool, parameters map[string]interface{}) error { + + fm.mutex.Lock() + + defer fm.mutex.Unlock() + + switch featureName { + + case "config": + + // update the AgentConfig with the new feature and it\'s configuration + + } + + return nil + +} +``` + +## Security Considerations + +- **Authentication & Authorisation**: Ensure that only authorised + entities can send gRPC messages to update feature flags. + +- **Validation**: Validate feature names and parameters received via + gRPC to prevent invalid configurations (e.g. using + ) + +- **Audit Logging**: Log all feature flag changes for auditing and + rollback purposes. + +- **Secret Management**: Securely handle sensitive configuration + parameters, especially for features like dealing with secrets and + connection settings. + +## Conclusion + +This design provides a flexible and dynamic feature flag management +system that integrates multiple configuration sources and allows +real-time updates via gRPC. + +By centralising feature state management and ensuring thread safety, the +system enables controlled feature toggling with minimal impact on the +running application. + +Proper security measures and validation ensure the integrity and +reliability of the feature management process. + + +[def]: #conflictingcombinations \ No newline at end of file diff --git a/internal/config/config.go b/internal/config/config.go index 4e581d3d27..514cf8ba88 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -102,6 +102,7 @@ func ResolveConfig() (*Config, error) { Command: resolveCommand(), Common: resolveCommon(), Watchers: resolveWatchers(), + Features: viperInstance.GetStringSlice(FeaturesKey), } slog.Debug("Agent config", "config", config) @@ -236,6 +237,12 @@ func registerFlags() { "Updates the client grpc setting MaxSendMsgSize with the specific value in MB.", ) + fs.StringSlice( + FeaturesKey, + GetDefaultFeatures(), + "A comma-separated list of features enabled for the agent.", + ) + registerCollectorFlags(fs) fs.SetNormalizeFunc(normalizeFunc) diff --git a/internal/config/defaults.go b/internal/config/defaults.go index 57d7903eb5..6c8ed93610 100644 --- a/internal/config/defaults.go +++ b/internal/config/defaults.go @@ -7,6 +7,8 @@ package config import ( "math" "time" + + pkg "github.com/nginx/agent/v3/pkg/config" ) const ( @@ -54,3 +56,12 @@ const ( DefCollectorBatchProcessorSendBatchMaxSize = 0 DefCollectorBatchProcessorTimeout = 200 * time.Millisecond ) + +func GetDefaultFeatures() []string { + return []string{ + pkg.FeatureConfiguration, + pkg.FeatureConnection, + pkg.FeatureMetrics, + pkg.FeatureFileWatcher, + } +} diff --git a/internal/config/flags.go b/internal/config/flags.go index 92a19839b1..9bf723bc55 100644 --- a/internal/config/flags.go +++ b/internal/config/flags.go @@ -18,6 +18,7 @@ const ( CollectorRootKey = "collector" VersionKey = "version" UUIDKey = "uuid" + FeaturesKey = "features" InstanceWatcherMonitoringFrequencyKey = "watchers_instance_watcher_monitoring_frequency" InstanceHealthWatcherMonitoringFrequencyKey = "watchers_instance_health_watcher_monitoring_frequency" FileWatcherMonitoringFrequencyKey = "watchers_file_watcher_monitoring_frequency" diff --git a/internal/config/types.go b/internal/config/types.go index 2765883048..3392e5b7a1 100644 --- a/internal/config/types.go +++ b/internal/config/types.go @@ -45,6 +45,7 @@ type ( ConfigDir string `yaml:"-" mapstructure:"config-dirs"` UUID string `yaml:"-"` AllowedDirectories []string `yaml:"-"` + Features []string `yaml:"-"` } Log struct { diff --git a/internal/watcher/instance/instance_watcher_service.go b/internal/watcher/instance/instance_watcher_service.go index 2840d0693b..958ad96866 100644 --- a/internal/watcher/instance/instance_watcher_service.go +++ b/internal/watcher/instance/instance_watcher_service.go @@ -299,7 +299,7 @@ func (iw *InstanceWatcherService) agentInstance(ctx context.Context) *mpi.Instan Metrics: &mpi.MetricsServer{}, File: &mpi.FileServer{}, Labels: []*structpb.Struct{}, - Features: []string{}, + Features: iw.agentConfig.Features, MessageBufferSize: "", }, }, diff --git a/nginx-agent.conf b/nginx-agent.conf index 141464fd7e..2109917eb6 100644 --- a/nginx-agent.conf +++ b/nginx-agent.conf @@ -5,25 +5,27 @@ # log: - # set log level (panic, fatal, error, info, debug, trace; default "info") + # set log level (error, info, debug; default "info") level: info # set log path. if empty, don't log to file. path: /var/log/nginx-agent/ -watchers: - instance_watcher: - monitoring_frequency: 5s - instance_health_watcher: - monitoring_frequency: 5s - file_watcher: - monitoring_frequency: 5s - -data_plane_config: - nginx: - reload_monitoring_period: 5s - treat_warnings_as_error: true - config_dirs: "/etc/nginx:/usr/local/etc/nginx:/usr/share/nginx/modules:/var/run/nginx:/var/log/nginx" -client: - timeout: 10s +## command server settings +# command: +# server: +# host: "127.0.0.1" # Command server host +# port: 8080 # Command server port +# type: 0 # Server type (e.g., 0 for gRPC) +# auth: +# token: "secret-auth-token" # Authentication token for the command server + +## collector metrics settings +# collector: +# exporters: # exporters +# - type: otlp # exporter type +# server: +# host: "127.0.0.1" # OTLP exporter server host +# port: 5643 # OTLP exporter server port +# tls: {} \ No newline at end of file diff --git a/pkg/config/features.go b/pkg/config/features.go new file mode 100644 index 0000000000..4bc857ca24 --- /dev/null +++ b/pkg/config/features.go @@ -0,0 +1,18 @@ +// 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 config + +const ( + FeatureCertificates = "certificates" + FeatureConfiguration = "configuration" + FeatureConnection = "connection" + FeatureMetrics = "metrics" + FeatureMetricsContainer = "metrics-container" + FeatureMetricsHost = "metrics-host" + FeatureMetricsInstance = "metrics-instance" + FeatureFileWatcher = "file-watcher" + FeatureAgentAPI = "agent-api" +)