Merge pull request #871 from fmuyassarov/disable-hook

Config option to disable hooks

deployment/components/worker-config/nfd-worker.conf.example

@@ -75,6 +75,8 @@
 #      - "class"
 #      - "vendor"
 #      - "device"
+#  local:
+#    hooksEnabled: true
 #  custom:
 #    # The following feature demonstrates the capabilities of the matchFeatures
 #    - name: "my custom rule"

deployment/helm/node-feature-discovery/values.yaml

@@ -169,6 +169,8 @@ worker:
     #      - "class"
     #      - "vendor"
     #      - "device"
+    #  local:
+    #    hooksEnabled: true
     #  custom:
     #    # The following feature demonstrates the capabilities of the matchFeatures
     #    - name: "my custom rule"

docs/advanced/customization-guide.md

@@ -153,6 +153,16 @@ should be placed in a separate directory in order to avoid NFD unnecessarily
 trying to execute them. A subdirectory under the hooks directory can be used,
 for example `/etc/kubernetes/node-feature-discovery/source.d/conf/`.
 
+**NOTE:** Hooks are being DEPRECATED and will be removed in a future release.
+For backward compatibility, currently hooks are enabled by default and can be
+disabled via `sources.local.hooksEnabled` field in the worker configuration.
+
+```yaml
+sources:
+  local:
+    hooksEnabled: true  # true by default at this point
+```
+
 **NOTE:** NFD will blindly run any executables placed/mounted in the hooks
 directory. It is the user's responsibility to review the hooks for e.g.
 possible security implications.

docs/advanced/worker-configuration-reference.md

@@ -329,6 +329,29 @@ sources:
     configOpts: [NO_HZ, X86, DMI]
 ```
 
+### sources.local
+
+### sources.local.hooksEnabled
+
+Configuration option to disable/enable hooks execution. Enabled by default.
+Hooks are DEPRECATED since v0.12.0 release and support will be removed in a
+future release. Use [feature files](./customization-guide.md#feature-files)
+instead.
+
+Related tracking issues:
+
+1. Config option to disable hooks [#859](https://github.com/kubernetes-sigs/node-feature-discovery/issues/859).
+1. Disable hook support by default [#855](https://github.com/kubernetes-sigs/node-feature-discovery/issues/855).
+1. Drop support for hooks [#856](https://github.com/kubernetes-sigs/node-feature-discovery/issues/856).
+
+Example:
+
+```yaml
+sources:
+  local:
+    hooksEnabled: true  # true by default
+```
+
 ### soures.pci
 
 #### soures.pci.deviceClassWhitelist

source/local/local.go

@@ -46,18 +46,40 @@ var (
 // localSource implements the FeatureSource and LabelSource interfaces.
 type localSource struct {
 	features *feature.DomainFeatures
+	config   *Config
+}
+
+type Config struct {
+	HooksEnabled bool `json:"hooksEnabled,omitempty"`
 }
 
 // Singleton source instance
 var (
-	src localSource
-	_   source.FeatureSource = &src
-	_   source.LabelSource   = &src
+	src                           = localSource{config: newDefaultConfig()}
+	_   source.FeatureSource      = &src
+	_   source.LabelSource        = &src
+	_   source.ConfigurableSource = &src
 )
 
 // Name method of the LabelSource interface
 func (s *localSource) Name() string { return Name }
 
+// NewConfig method of the LabelSource interface
+func (s *localSource) NewConfig() source.Config { return newDefaultConfig() }
+
+// GetConfig method of the LabelSource interface
+func (s *localSource) GetConfig() source.Config { return s.config }
+
+// SetConfig method of the LabelSource interface
+func (s *localSource) SetConfig(conf source.Config) {
+	switch v := conf.(type) {
+	case *Config:
+		s.config = v
+	default:
+		klog.Fatalf("invalid config type: %T", conf)
+	}
+}
+
 // Priority method of the LabelSource interface
 func (s *localSource) Priority() int { return 20 }
 
@@ -72,28 +94,41 @@ func (s *localSource) GetLabels() (source.FeatureLabels, error) {
 	return labels, nil
 }
 
+// newDefaultConfig returns a new config with pre-populated defaults
+func newDefaultConfig() *Config {
+	return &Config{
+		HooksEnabled: true,
+	}
+}
+
 // Discover method of the FeatureSource interface
 func (s *localSource) Discover() error {
 	s.features = feature.NewDomainFeatures()
 
-	featuresFromHooks, err := getFeaturesFromHooks()
-	if err != nil {
-		klog.Error(err)
-	}
-
 	featuresFromFiles, err := getFeaturesFromFiles()
 	if err != nil {
 		klog.Error(err)
 	}
 
-	// Merge features from hooks and files
-	for k, v := range featuresFromHooks {
-		if old, ok := featuresFromFiles[k]; ok {
-			klog.Warningf("overriding '%s': value changed from '%s' to '%s'",
-				k, old, v)
+	if s.config.HooksEnabled {
+
+		klog.Info("starting hooks...")
+
+		featuresFromHooks, err := getFeaturesFromHooks()
+		if err != nil {
+			klog.Error(err)
+		}
+
+		// Merge features from hooks and files
+		for k, v := range featuresFromHooks {
+			if old, ok := featuresFromFiles[k]; ok {
+				klog.Warningf("overriding '%s': value changed from '%s' to '%s'",
+					k, old, v)
+			}
+			featuresFromFiles[k] = v
 		}
-		featuresFromFiles[k] = v
 	}
+
 	s.features.Values[LabelFeature] = feature.NewValueFeatures(featuresFromFiles)
 
 	utils.KlogDump(3, "discovered local features:", "  ", s.features)
@@ -132,6 +167,7 @@ func parseFeatures(lines [][]byte) map[string]string {
 
 // Run all hooks and get features
 func getFeaturesFromHooks() (map[string]string, error) {
+
 	features := make(map[string]string)
 
 	files, err := os.ReadDir(hookDir)
@@ -142,6 +178,9 @@ func getFeaturesFromHooks() (map[string]string, error) {
 		}
 		return features, fmt.Errorf("unable to access %v: %v", hookDir, err)
 	}
+	if len(files) > 0 {
+		klog.Warning("hooks are DEPRECATED since v0.12.0 and support will be removed in a future release; use feature files instead")
+	}
 
 	for _, file := range files {
 		fileName := file.Name()