Enable disable features (#300)

* enable disable features --------- Co-authored-by: dhurley <[email protected]> Co-authored-by: Dean Coakley <[email protected]>
nginx · Jun 6, 2023 · 5974615 · 5974615
1 parent 32ef235
commit 5974615
Show file tree

Hide file tree

Showing 58 changed files with 2,023 additions and 339 deletions.
diff --git a/Makefile b/Makefile
@@ -105,7 +105,7 @@ deps: ## Update dependencies in vendor folders
 	go work sync
 
 no-local-changes:
-	git diff --quiet || { echo "Depenency changes detected. Please commit these before pushing." >&2; exit 1; }
+	git diff --quiet || { echo "Dependency changes detected. Please commit these before pushing." >&2; exit 1; }
 
 lint: ## Run linter
 	GOWORK=off go vet ./...

diff --git a/main.go b/main.go
@@ -17,11 +17,7 @@ import (
 	"syscall"
 	"time"
 
-	"github.com/google/uuid"
-	log "github.com/sirupsen/logrus"
-	"github.com/spf13/cobra"
-	"google.golang.org/grpc"
-
+	agent_config "github.com/nginx/agent/sdk/v2/agent/config"
 	"github.com/nginx/agent/sdk/v2/client"
 	sdkGRPC "github.com/nginx/agent/sdk/v2/grpc"
 	"github.com/nginx/agent/v2/src/core"
@@ -30,7 +26,10 @@ import (
 	"github.com/nginx/agent/v2/src/extensions"
 	"github.com/nginx/agent/v2/src/plugins"
 
-	agent_config "github.com/nginx/agent/sdk/v2/agent/config"
+	"github.com/google/uuid"
+	log "github.com/sirupsen/logrus"
+	"github.com/spf13/cobra"
+	"google.golang.org/grpc"
 )
 
 var (
@@ -148,7 +147,7 @@ func handleSignals(
 
 func createGrpcClients(ctx context.Context, loadedConfig *config.Config) (client.Controller, client.Commander, client.MetricReporter) {
 	if !loadedConfig.IsGrpcServerConfigured() {
-		log.Infof("GRPC clients not created")
+		log.Info("GRPC clients not created due to missing server config")
 		return nil, nil, nil
 	}
 
@@ -199,9 +198,14 @@ func loadPlugins(commander client.Commander, binary *core.NginxBinaryType, env *
 	if commander != nil {
 		corePlugins = append(corePlugins,
 			plugins.NewCommander(commander, loadedConfig),
-			plugins.NewFileWatcher(loadedConfig, env),
-			plugins.NewFileWatchThrottle(),
 		)
+
+		if loadedConfig.IsFeatureEnabled(agent_config.FeatureFileWatcher) {
+			corePlugins = append(corePlugins,
+				plugins.NewFileWatcher(loadedConfig, env),
+				plugins.NewFileWatchThrottle(),
+			)
+		}
 	}
 
 	if reporter != nil {
@@ -213,22 +217,41 @@ func loadPlugins(commander client.Commander, binary *core.NginxBinaryType, env *
 	corePlugins = append(corePlugins,
 		plugins.NewConfigReader(loadedConfig),
 		plugins.NewNginx(commander, binary, env, loadedConfig),
-		plugins.NewOneTimeRegistration(loadedConfig, binary, env, sdkGRPC.NewMessageMeta(uuid.NewString()), version),
-		plugins.NewMetrics(loadedConfig, env, binary),
-		plugins.NewMetricsThrottle(loadedConfig, env),
-		plugins.NewDataPlaneStatus(loadedConfig, sdkGRPC.NewMessageMeta(uuid.NewString()), binary, env, version),
-		plugins.NewProcessWatcher(env, binary),
 		plugins.NewExtensions(loadedConfig, env),
-		plugins.NewEvents(loadedConfig, env, sdkGRPC.NewMessageMeta(uuid.NewString()), binary),
+		plugins.NewFeatures(commander, loadedConfig, env, binary, version),
 	)
 
-	if loadedConfig.AgentAPI.Port != 0 {
+	if loadedConfig.IsFeatureEnabled(agent_config.FeatureRegistration) {
+		corePlugins = append(corePlugins, plugins.NewOneTimeRegistration(loadedConfig, binary, env, sdkGRPC.NewMessageMeta(uuid.NewString()), version))
+	}
+
+	if loadedConfig.IsFeatureEnabled(agent_config.FeatureMetrics) || (len(loadedConfig.Nginx.NginxCountingSocket) > 0 && loadedConfig.IsFeatureEnabled(agent_config.FeatureNginxCounting)) {
+		corePlugins = append(corePlugins, plugins.NewMetrics(loadedConfig, env, binary))
+	}
+
+	if loadedConfig.IsFeatureEnabled(agent_config.FeatureMetricsThrottle) {
+		corePlugins = append(corePlugins, plugins.NewMetricsThrottle(loadedConfig, env))
+	}
+
+	if loadedConfig.IsFeatureEnabled(agent_config.FeatureDataPlaneStatus) {
+		corePlugins = append(corePlugins, plugins.NewDataPlaneStatus(loadedConfig, sdkGRPC.NewMessageMeta(uuid.NewString()), binary, env, version))
+	}
+
+	if loadedConfig.IsFeatureEnabled(agent_config.FeatureProcessWatcher) {
+		corePlugins = append(corePlugins, plugins.NewProcessWatcher(env, binary))
+	}
+
+	if loadedConfig.IsFeatureEnabled(agent_config.FeatureActivityEvents) {
+		corePlugins = append(corePlugins, plugins.NewEvents(loadedConfig, env, sdkGRPC.NewMessageMeta(uuid.NewString()), binary))
+	}
+
+	if loadedConfig.AgentAPI.Port != 0 && loadedConfig.IsFeatureEnabled(agent_config.FeatureAgentAPI) {
 		corePlugins = append(corePlugins, plugins.NewAgentAPI(loadedConfig, env, binary))
 	} else {
 		log.Info("Agent API not configured")
 	}
 
-	if len(loadedConfig.Nginx.NginxCountingSocket) > 0 {
+	if len(loadedConfig.Nginx.NginxCountingSocket) > 0 && loadedConfig.IsFeatureEnabled(agent_config.FeatureNginxCounting) {
 		corePlugins = append(corePlugins, plugins.NewNginxCounter(loadedConfig, binary, env))
 	}
 

diff --git a/sdk/agent/config/config_helpers.go b/sdk/agent/config/config_helpers.go
@@ -15,20 +15,22 @@ const (
 	DefaultPluginSize = 100
 	KeyDelimiter      = "_"
 
-	// Features
-	FeaturesKey             = "features"
-	FeatureRegistration     = FeaturesKey + KeyDelimiter + "registration"
-	FeatureNginxConfig      = FeaturesKey + KeyDelimiter + "nginx-config"
-	FeatureNginxConfigAsync = FeaturesKey + KeyDelimiter + "nginx-config-async"
-	FeatureNginxSSLConfig   = FeaturesKey + KeyDelimiter + "nginx-ssl-config"
-	FeatureNginxCounting    = FeaturesKey + KeyDelimiter + "nginx-counting"
-	FeatureMetrics          = FeaturesKey + KeyDelimiter + "metrics"
-	FeatureMetricsThrottle  = FeaturesKey + KeyDelimiter + "metrics-throttle"
-	FeatureDataPlaneStatus  = FeaturesKey + KeyDelimiter + "dataplane-status"
-	FeatureProcessWatcher   = FeaturesKey + KeyDelimiter + "process-watcher"
-	FeatureFileWatcher      = FeaturesKey + KeyDelimiter + "file-watcher"
-	FeatureActivityEvents   = FeaturesKey + KeyDelimiter + "activity-events"
-	FeatureAgentAPI         = FeaturesKey + KeyDelimiter + "agent-api"
+	// viper keys used in config
+	FeaturesKey         = "features"
+	FeatureRegistration = "registration"
+	// Deprecated: use nginx-config-async instead
+	FeatureNginxConfig         = "nginx-config"
+	FeatureNginxConfigAsync    = "nginx-config-async"
+	FeatureNginxSSLConfig      = "nginx-ssl-config"
+	FeatureNginxCounting       = "nginx-counting"
+	FeatureMetrics             = "metrics"
+	FeatureMetricsThrottle     = "metrics-throttle"
+	FeatureDataPlaneStatus     = "dataplane-status"
+	FeatureProcessWatcher      = "process-watcher"
+	FeatureFileWatcher         = "file-watcher"
+	FeatureFileWatcherThrottle = "file-watch-throttle"
+	FeatureActivityEvents      = "activity-events"
+	FeatureAgentAPI            = "agent-api"
 
 	// Extensions
 	ExtensionsKey                            = "extensions"
@@ -63,10 +65,9 @@ func IsKnownExtension(extension string) bool {
 func GetDefaultFeatures() []string {
 	return []string{
 		FeatureRegistration,
-		FeatureNginxConfig,
+		FeatureNginxConfigAsync,
 		FeatureNginxSSLConfig,
 		FeatureNginxCounting,
-		FeatureNginxConfigAsync,
 		FeatureMetrics,
 		FeatureMetricsThrottle,
 		FeatureDataPlaneStatus,

diff --git a/src/core/config/config.go b/src/core/config/config.go
@@ -8,8 +8,12 @@
 package config
 
 import (
+	"bufio"
+	"bytes"
+	"errors"
 	"fmt"
-	"io/ioutil"
+	"io"
+	"io/fs"
 	"os"
 	"path/filepath"
 	"reflect"
@@ -20,12 +24,11 @@ import (
 
 	agent_config "github.com/nginx/agent/sdk/v2/agent/config"
 
+	log "github.com/sirupsen/logrus"
 	"github.com/spf13/cobra"
+	flag "github.com/spf13/pflag"
 	"github.com/spf13/viper"
 	"gopkg.in/yaml.v3"
-
-	log "github.com/sirupsen/logrus"
-	flag "github.com/spf13/pflag"
 )
 
 const (
@@ -219,6 +222,12 @@ func UpdateAgentConfig(systemId string, updateTags []string, updateFeatures []st
 	Viper.Set(TagsKey, updateTags)
 	config.Tags = Viper.GetStringSlice(TagsKey)
 
+	// Needed for legacy reasons.
+	// Remove Features_ prefix from the feature strings.
+	// This is needed for management servers that are sending features before sdk version v2.23.0
+	for index, feature := range updateFeatures {
+		updateFeatures[index] = strings.Replace(feature, "features_", "", 1)
+	}
 	sort.Strings(updateFeatures)
 	sort.Strings(config.Features)
 	synchronizedFeatures := reflect.DeepEqual(updateFeatures, config.Features)
@@ -251,7 +260,7 @@ func UpdateAgentConfig(systemId string, updateTags []string, updateFeatures []st
 
 	updatedConfBytes = append([]byte(dynamicConfigUsageComment), updatedConfBytes...)
 
-	err = ioutil.WriteFile(dynamicCfgPath, updatedConfBytes, 0)
+	err = os.WriteFile(dynamicCfgPath, updatedConfBytes, 0)
 	if err != nil {
 		return false, err
 	}
@@ -357,25 +366,46 @@ func LoadPropertiesFromFile(cfg string) error {
 			dynamicCfgPath = DynamicConfigFileAbsPath
 		}
 	}
+
 	dynamicCfgDir, dynamicCfgFile := filepath.Split(dynamicCfgPath)
 
 	// Get dynamic file, if it doesn't exist create it.
-	file, err := os.Stat(dynamicCfgPath)
-	if err != nil {
-		log.Warnf("Unable to read dynamic config (%s), got the following error: %v", dynamicCfgPath, err)
-	}
+	_, err = os.Stat(dynamicCfgPath)
+	if err == nil {
+		log.Debugf("Checking if features need to be purged from dynamic config: %s", dynamicCfgPath)
+
+		dynCfg, err := os.Open(dynamicCfgPath)
+		if err != nil {
+			return fmt.Errorf("error attempting to open dynamic config (%s): %v", dynamicCfgPath, err)
+		}
+
+		featuresAreSet, cleanDynCfgContent, err := removeFeatures(dynCfg)
+		if err != nil {
+			return fmt.Errorf("error updating dynamic config with features removed (%s): %v", dynamicCfgPath, err)
+		}
+		dynCfg.Close()
 
-	if file == nil {
+		if featuresAreSet {
+			err = os.WriteFile(dynamicCfgPath, cleanDynCfgContent, 0644)
+			if err != nil {
+				return fmt.Errorf("error attempting to update dynamic config (%s): %v", dynamicCfgPath, err)
+			}
+
+			log.Info("Dynamic config purged successfully. Previously enabled features have been removed")
+		}
+	} else if errors.Is(err, fs.ErrNotExist) {
 		log.Infof("Writing the following file to disk: %s", dynamicCfgPath)
 		err = os.MkdirAll(dynamicCfgDir, 0755)
 		if err != nil {
-			return fmt.Errorf("error attempting to create directory for dynamic config (%s), got the following error: %v", dynamicCfgDir, err)
+			return fmt.Errorf("error attempting to create directory for dynamic config (%s): %v", dynamicCfgDir, err)
 		}
 
 		err = os.WriteFile(dynamicCfgPath, []byte(dynamicConfigUsageComment), 0644)
 		if err != nil {
-			return fmt.Errorf("error attempting to create dynamic config (%s), got the following error: %v", dynamicCfgPath, err)
+			return fmt.Errorf("error attempting to create dynamic config (%s): %v", dynamicCfgPath, err)
 		}
+	} else if err != nil {
+		log.Warnf("Unable to read dynamic config (%s): %v", dynamicCfgPath, err)
 	}
 
 	// Load properties from existing file
@@ -390,6 +420,45 @@ func LoadPropertiesFromFile(cfg string) error {
 	return nil
 }
 
+// removeFeatures removes enabled features from dynamic config content
+func removeFeatures(readFile io.Reader) (bool, []byte, error) {
+	fileScanner := bufio.NewScanner(readFile)
+
+	fileScanner.Split(bufio.ScanLines)
+
+	var bs []byte
+	buf := bytes.NewBuffer(bs)
+
+	var featuresSet bool
+
+	for fileScanner.Scan() {
+		if strings.HasPrefix(fileScanner.Text(), "features") {
+			featuresSet = true
+			for fileScanner.Scan() {
+				if !strings.HasPrefix(strings.TrimSpace(fileScanner.Text()), "-") {
+					_, err := buf.Write(fileScanner.Bytes())
+					if err != nil {
+						return featuresSet, nil, err
+					}
+					break
+				}
+			}
+		} else {
+			_, err := buf.Write(fileScanner.Bytes())
+			if err != nil {
+				return featuresSet, nil, err
+			}
+		}
+
+		_, err := buf.WriteString("\n")
+		if err != nil {
+			return featuresSet, nil, err
+		}
+	}
+
+	return featuresSet, buf.Bytes(), nil
+}
+
 func SetDynamicConfigFileAbsPath(dynamicCfgPath string) {
 	Viper.Set(DynamicConfigPathKey, dynamicCfgPath)
 	log.Debugf("Set dynamic agent config file: %s", dynamicCfgPath)