chore: make spiffe optional

Signed-off-by: AhmedGrati <[email protected]>

Makefile

@@ -120,21 +120,21 @@ deploy: yamls
 templates:
 	@# Need to prepend each line in the sample config with spaces in order to
 	@# fit correctly in the configmap spec.
-	@sed s'/^/    /' deployment/components/worker-config/nfd-worker.conf.example > nfd-worker.conf.tmp
-	@sed s'/^/    /' deployment/components/master-config/nfd-master.conf.example > nfd-master.conf.tmp
-	@sed s'/^/    /' deployment/components/topology-updater-config/nfd-topology-updater.conf.example > nfd-topology-updater.conf.tmp
+	@gsed s'/^/    /' deployment/components/worker-config/nfd-worker.conf.example > nfd-worker.conf.tmp
+	@gsed s'/^/    /' deployment/components/master-config/nfd-master.conf.example > nfd-master.conf.tmp
+	@gsed s'/^/    /' deployment/components/topology-updater-config/nfd-topology-updater.conf.example > nfd-topology-updater.conf.tmp
 	@# The sed magic below replaces the block of text between the lines with start and end markers
 	@start=NFD-MASTER-CONF-START-DO-NOT-REMOVE; \
 	end=NFD-MASTER-CONF-END-DO-NOT-REMOVE; \
-	sed -e "/$$start/,/$$end/{ /$$start/{ p; r nfd-master.conf.tmp" \
+	gsed -e "/$$start/,/$$end/{ /$$start/{ p; r nfd-master.conf.tmp" \
 	    -e "}; /$$end/p; d }" -i deployment/helm/node-feature-discovery/values.yaml
 	@start=NFD-WORKER-CONF-START-DO-NOT-REMOVE; \
 	end=NFD-WORKER-CONF-END-DO-NOT-REMOVE; \
-	sed -e "/$$start/,/$$end/{ /$$start/{ p; r nfd-worker.conf.tmp" \
+	gsed -e "/$$start/,/$$end/{ /$$start/{ p; r nfd-worker.conf.tmp" \
 	    -e "}; /$$end/p; d }" -i deployment/helm/node-feature-discovery/values.yaml
 	@start=NFD-TOPOLOGY-UPDATER-CONF-START-DO-NOT-REMOVE; \
 	end=NFD-TOPOLOGY-UPDATER-CONF-END-DO-NOT-REMOVE; \
-	sed -e "/$$start/,/$$end/{ /$$start/{ p; r nfd-topology-updater.conf.tmp" \
+	gsed -e "/$$start/,/$$end/{ /$$start/{ p; r nfd-topology-updater.conf.tmp" \
 		-e "}; /$$end/p; d }" -i deployment/helm/node-feature-discovery/values.yaml
 	@rm nfd-master.conf.tmp
 	@rm nfd-worker.conf.tmp

cmd/nfd-master/main.go

@@ -71,6 +71,8 @@ func main() {
 			args.Overrides.ResyncPeriod = overrides.ResyncPeriod
 		case "nfd-api-parallelism":
 			args.Overrides.NfdApiParallelism = overrides.NfdApiParallelism
+		case "enable-spiffe":
+			args.Overrides.EnableSpiffe = overrides.EnableSpiffe
 		case "enable-nodefeature-api":
 			klog.InfoS("-enable-nodefeature-api is deprecated, will be removed in a future release along with the deprecated gRPC API")
 		case "ca-file":
@@ -181,6 +183,7 @@ func initFlags(flagset *flag.FlagSet) (*master.Args, *master.ConfigOverrideArgs)
 			"It has an effect when the NodeFeature API has been enabled (with -enable-nodefeature-api).")
 	overrides.NfdApiParallelism = flagset.Int("nfd-api-parallelism", 10, "Defines the maximum number of goroutines responsible of updating nodes. "+
 		"Can be used for the throttling mechanism. It has effect only when -enable-nodefeature-api has been set.")
-
+	overrides.EnableSpiffe = flagset.Bool("enable-spiffe", false,
+		"Enables the Spiffe signature verification of created CRDs. This is still an EXPERIMENTAL feature.")
 	return args, overrides
 }

cmd/nfd-worker/main.go

@@ -104,6 +104,8 @@ func parseArgs(flags *flag.FlagSet, osArgs ...string) *worker.Args {
 			args.Overrides.FeatureSources = overrides.FeatureSources
 		case "label-sources":
 			args.Overrides.LabelSources = overrides.LabelSources
+		case "enable-spiffe":
+			args.Overrides.EnableSpiffe = overrides.EnableSpiffe
 		}
 	})
 
@@ -158,6 +160,8 @@ func initFlags(flagset *flag.FlagSet) (*worker.Args, *worker.ConfigOverrideArgs)
 	flagset.Var(overrides.LabelSources, "label-sources",
 		"Comma separated list of label sources. Special value 'all' enables all sources. "+
 			"Prefix the source name with '-' to disable it.")
+	overrides.EnableSpiffe = flagset.Bool("enable-spiffe", false,
+		"Enables the Spiffe signature verification of created CRDs. This is still an EXPERIMENTAL feature.")
 
 	return args, overrides
 }

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

@@ -3,6 +3,7 @@
 # denyLabelNs: ["denied.ns.io","denied.kubernetes.io"]
 # resourceLabels: ["vendor-1.com/feature-1","vendor-2.io/feature-2"]
 # enableTaints: false
+# enableSpiffe: true
 # labelWhiteList: "foo"
 # resyncPeriod: "2h"
 # klog:

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

@@ -2,6 +2,7 @@
 #  labelWhiteList:
 #  noPublish: false
 #  sleepInterval: 60s
+#  enableSpiffe: true
 #  featureSources: [all]
 #  labelSources: [all]
 #  klog:

deployment/helm/node-feature-discovery/templates/master.yaml

@@ -108,6 +108,9 @@ spec:
             - "-key-file=/etc/kubernetes/node-feature-discovery/certs/tls.key"
             - "-cert-file=/etc/kubernetes/node-feature-discovery/certs/tls.crt"
             {{- end }}
+            {{- if .Values.spiffe.enable }}
+            - "-enable-spiffe"
+            {{- end }}
             - "-metrics={{ .Values.master.metricsPort  | default "8081" }}"
           volumeMounts:
             {{- if .Values.tls.enable }}

deployment/helm/node-feature-discovery/templates/spire-agent-cluster-role.yaml

@@ -17,7 +17,7 @@ metadata:
 subjects:
 - kind: ServiceAccount
   name: spire-agent
-  namespace: spire
+  namespace: {{ include "node-feature-discovery.namespace" . }}
 roleRef:
   kind: ClusterRole
   name: spire-agent-cluster-role

deployment/helm/node-feature-discovery/templates/spire-server-cluster-role.yaml

@@ -4,7 +4,6 @@ kind: Role
 apiVersion: rbac.authorization.k8s.io/v1
 metadata:
   name: spire-server-configmap-role
-  namespace: spire
 rules:
 - apiGroups: [""]
   resources: ["configmaps"]
@@ -15,11 +14,11 @@ kind: RoleBinding
 apiVersion: rbac.authorization.k8s.io/v1
 metadata:
   name: spire-server-configmap-role-binding
-  namespace: spire
+  namespace: {{ include "node-feature-discovery.namespace" . }}
 subjects:
 - kind: ServiceAccount
   name: spire-server
-  namespace: spire
+  namespace: {{ include "node-feature-discovery.namespace" . }}
 roleRef:
   apiGroup: rbac.authorization.k8s.io
   kind: Role
@@ -43,7 +42,7 @@ metadata:
 subjects:
 - kind: ServiceAccount
   name: spire-server
-  namespace: spire
+  namespace: {{ include "node-feature-discovery.namespace" . }}
 roleRef:
   kind: ClusterRole
   name: spire-server-trust-role

deployment/helm/node-feature-discovery/templates/spire-server-statefulset.yaml

@@ -13,7 +13,6 @@ spec:
   serviceName: spire-server
   template:
     metadata:
-      namespace: spire
       labels:
         app: spire-server
     spec:
@@ -54,7 +53,6 @@ spec:
   volumeClaimTemplates:
     - metadata:
         name: spire-data
-        namespace: spire
       spec:
         accessModes:
           - ReadWriteOnce

deployment/helm/node-feature-discovery/templates/worker.yaml

@@ -59,6 +59,9 @@ spec:
         - "-key-file=/etc/kubernetes/node-feature-discovery/certs/tls.key"
         - "-cert-file=/etc/kubernetes/node-feature-discovery/certs/tls.crt"
 {{- end }}
+        {{- if .Values.spiffe.enable }}
+        - "-enable-spiffe"
+        {{- end }}
         - "-metrics={{ .Values.worker.metricsPort | default "8081"}}"
         ports:
           - name: metrics

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

@@ -24,8 +24,8 @@ master:
     # denyLabelNs: ["denied.ns.io","denied.kubernetes.io"]
     # resourceLabels: ["vendor-1.com/feature-1","vendor-2.io/feature-2"]
     # enableTaints: false
-    # labelWhiteList: "foo"
     # enableSpiffe: true
+    # labelWhiteList: "foo"
     # resyncPeriod: "2h"
     # klog:
     #    addDirHeader: false
@@ -144,9 +144,9 @@ worker:
     #  labelWhiteList:
     #  noPublish: false
     #  sleepInterval: 60s
+    #  enableSpiffe: true
     #  featureSources: [all]
     #  labelSources: [all]
-    #  enableSpiffe: true
     #  klog:
     #    addDirHeader: false
     #    alsologtostderr: false

deployment/overlays/spiffe/spire-agent-cluster-role.yaml

@@ -18,7 +18,7 @@ metadata:
 subjects:
 - kind: ServiceAccount
   name: spire-agent
-  namespace: spire
+  namespace: {{ include "node-feature-discovery.namespace" . }}
 roleRef:
   kind: ClusterRole
   name: spire-agent-cluster-role

deployment/overlays/spiffe/spire-server-cluster-role.yaml

@@ -3,7 +3,6 @@ kind: Role
 apiVersion: rbac.authorization.k8s.io/v1
 metadata:
   name: spire-server-configmap-role
-  namespace: spire
 rules:
 - apiGroups: [""]
   resources: ["configmaps"]
@@ -14,11 +13,11 @@ kind: RoleBinding
 apiVersion: rbac.authorization.k8s.io/v1
 metadata:
   name: spire-server-configmap-role-binding
-  namespace: spire
+  namespace: {{ include "node-feature-discovery.namespace" . }}
 subjects:
 - kind: ServiceAccount
   name: spire-server
-  namespace: spire
+  namespace: {{ include "node-feature-discovery.namespace" . }}
 roleRef:
   apiGroup: rbac.authorization.k8s.io
   kind: Role
@@ -42,7 +41,7 @@ metadata:
 subjects:
 - kind: ServiceAccount
   name: spire-server
-  namespace: spire
+  namespace: {{ include "node-feature-discovery.namespace" . }}
 roleRef:
   kind: ClusterRole
   name: spire-server-trust-role

deployment/overlays/spiffe/spire-server-statefulset.yaml

@@ -12,7 +12,6 @@ spec:
   serviceName: spire-server
   template:
     metadata:
-      namespace: spire
       labels:
         app: spire-server
     spec:
@@ -53,7 +52,6 @@ spec:
   volumeClaimTemplates:
     - metadata:
         name: spire-data
-        namespace: spire
       spec:
         accessModes:
           - ReadWriteOnce

pkg/nfd-master/nfd-master.go

@@ -86,6 +86,7 @@ type NFDConfig struct {
 	LeaderElection    LeaderElectionConfig
 	NfdApiParallelism int
 	Klog              klogutils.KlogConfigOpts
+	EnableSpiffe      bool
 }
 
 // LeaderElectionConfig contains the configuration for leader election
@@ -105,6 +106,7 @@ type ConfigOverrideArgs struct {
 	NoPublish         *bool
 	ResyncPeriod      *utils.DurationVal
 	NfdApiParallelism *int
+	EnableSpiffe      *bool
 }
 
 // Args holds command line arguments
@@ -211,6 +213,7 @@ func newDefaultConfig() *NFDConfig {
 		NfdApiParallelism: 10,
 		ResourceLabels:    utils.StringSetVal{},
 		EnableTaints:      false,
+		EnableSpiffe:      false,
 		ResyncPeriod:      utils.DurationVal{Duration: time.Duration(1) * time.Hour},
 		LeaderElection: LeaderElectionConfig{
 			LeaseDuration: utils.DurationVal{Duration: time.Duration(15) * time.Second},
@@ -764,19 +767,11 @@ func (m *nfdMaster) nfdAPIUpdateOneNode(nodeName string) error {
 		return objs[i].Namespace < objs[j].Namespace
 	})
 
-	verifiedObjects := []*v1alpha1.NodeFeature{}
-	// Verify nfd objects signature
-	for _, obj := range objs {
-		isSignatureVerified, err := m.spiffeClient.VerifyDataSignature(obj.Spec, spiffe.GetSpiffeId(utils.NodeName()), obj.Annotations["signature"])
+	// If spiffe is enabled, we should filter out the non verified NFD objects
+	if m.config.EnableSpiffe {
+		objs, err = m.getVerifiedNFDObjects(objs)
 		if err != nil {
-			klog.ErrorS(err, "error while getting data signature")
-			return fmt.Errorf("failed to sign CRD data using Spiffe: %w", err)
-		}
-		if isSignatureVerified {
-			klog.InfoS("data verified", "nfd name", obj.Name)
-			verifiedObjects = append(verifiedObjects, obj)
-		} else {
-			klog.InfoS("data not verified", "nfd name", obj.Name)
+			return err
 		}
 	}
 
@@ -790,13 +785,13 @@ func (m *nfdMaster) nfdAPIUpdateOneNode(nodeName string) error {
 
 	annotations := Annotations{}
 
-	if len(verifiedObjects) > 0 {
+	if len(objs) > 0 {
 		// Merge in features
 		//
 		// NOTE: changing the rule api to support handle multiple objects instead
 		// of merging would probably perform better with lot less data to copy.
-		features = verifiedObjects[0].Spec.DeepCopy()
-		for _, o := range verifiedObjects[1:] {
+		features = objs[0].Spec.DeepCopy()
+		for _, o := range objs[1:] {
 			o.Spec.MergeInto(features)
 		}
 
@@ -1261,6 +1256,9 @@ func (m *nfdMaster) configure(filepath string, overrides string) error {
 	if m.args.Overrides.NfdApiParallelism != nil {
 		c.NfdApiParallelism = *m.args.Overrides.NfdApiParallelism
 	}
+	if m.args.Overrides.EnableSpiffe != nil {
+		c.EnableSpiffe = *m.args.Overrides.EnableSpiffe
+	}
 
 	if c.NfdApiParallelism <= 0 {
 		return fmt.Errorf("the maximum number of concurrent labelers should be a non-zero positive number")
@@ -1407,3 +1405,22 @@ func (m *nfdMaster) nfdAPIUpdateHandlerWithLeaderElection() {
 
 	leaderElector.Run(ctx)
 }
+
+func (m *nfdMaster) getVerifiedNFDObjects(objs []*v1alpha1.NodeFeature) ([]*v1alpha1.NodeFeature, error) {
+	verifiedObjects := []*v1alpha1.NodeFeature{}
+
+	for _, obj := range objs {
+		isSignatureVerified, err := m.spiffeClient.VerifyDataSignature(obj.Spec, spiffe.GetSpiffeId(utils.NodeName()), obj.Annotations["signature"])
+		if err != nil {
+			return nil, fmt.Errorf("failed to verify NodeFeature signature: %w", err)
+		}
+
+		if isSignatureVerified {
+			klog.InfoS("NodeFeature verified", "NodeFeature name", obj.Name)
+			verifiedObjects = append(verifiedObjects, obj)
+		} else {
+			klog.InfoS("NodeFeature not verified, skipping...", "NodeFeature name", obj.Name)
+		}
+	}
+	return verifiedObjects, nil
+}