From da71ef4166cfecd8be22e73953e90fc583f9b119 Mon Sep 17 00:00:00 2001 From: AhmedGrati Date: Sun, 29 Oct 2023 16:28:37 +0100 Subject: [PATCH] chore: make spiffe optional Signed-off-by: AhmedGrati --- Makefile | 12 ++--- cmd/nfd-master/main.go | 5 +- cmd/nfd-worker/main.go | 4 ++ .../master-config/nfd-master.conf.example | 1 + .../worker-config/nfd-worker.conf.example | 1 + .../templates/master.yaml | 3 ++ .../templates/worker.yaml | 3 ++ .../helm/node-feature-discovery/values.yaml | 4 +- pkg/nfd-master/nfd-master.go | 47 +++++++++++++------ pkg/nfd-worker/nfd-worker.go | 37 +++++++++------ 10 files changed, 78 insertions(+), 39 deletions(-) diff --git a/Makefile b/Makefile index 2fe8b85b79..edcd621ad3 100644 --- a/Makefile +++ b/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 diff --git a/cmd/nfd-master/main.go b/cmd/nfd-master/main.go index 3418d2af14..6174505b91 100644 --- a/cmd/nfd-master/main.go +++ b/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 } diff --git a/cmd/nfd-worker/main.go b/cmd/nfd-worker/main.go index 6093484783..0c56a98632 100644 --- a/cmd/nfd-worker/main.go +++ b/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 } diff --git a/deployment/components/master-config/nfd-master.conf.example b/deployment/components/master-config/nfd-master.conf.example index 11d320272b..1199f1426a 100644 --- a/deployment/components/master-config/nfd-master.conf.example +++ b/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: diff --git a/deployment/components/worker-config/nfd-worker.conf.example b/deployment/components/worker-config/nfd-worker.conf.example index b37878c763..850a8b95fa 100644 --- a/deployment/components/worker-config/nfd-worker.conf.example +++ b/deployment/components/worker-config/nfd-worker.conf.example @@ -2,6 +2,7 @@ # labelWhiteList: # noPublish: false # sleepInterval: 60s +# enableSpiffe: true # featureSources: [all] # labelSources: [all] # klog: diff --git a/deployment/helm/node-feature-discovery/templates/master.yaml b/deployment/helm/node-feature-discovery/templates/master.yaml index a2e39a8757..ed5f3a5d71 100644 --- a/deployment/helm/node-feature-discovery/templates/master.yaml +++ b/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 }} diff --git a/deployment/helm/node-feature-discovery/templates/worker.yaml b/deployment/helm/node-feature-discovery/templates/worker.yaml index 65945b0db7..b42c26970b 100644 --- a/deployment/helm/node-feature-discovery/templates/worker.yaml +++ b/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 diff --git a/deployment/helm/node-feature-discovery/values.yaml b/deployment/helm/node-feature-discovery/values.yaml index 6848177644..c918c1f9f5 100644 --- a/deployment/helm/node-feature-discovery/values.yaml +++ b/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 diff --git a/pkg/nfd-master/nfd-master.go b/pkg/nfd-master/nfd-master.go index 3563aa8248..377d1e4776 100644 --- a/pkg/nfd-master/nfd-master.go +++ b/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 +} diff --git a/pkg/nfd-worker/nfd-worker.go b/pkg/nfd-worker/nfd-worker.go index 118adbfac6..5626febb4d 100644 --- a/pkg/nfd-worker/nfd-worker.go +++ b/pkg/nfd-worker/nfd-worker.go @@ -87,6 +87,7 @@ type coreConfig struct { Sources *[]string LabelSources []string SleepInterval utils.DurationVal + EnableSpiffe bool } type sourcesConfig map[string]source.Config @@ -118,6 +119,7 @@ type ConfigOverrideArgs struct { FeatureSources *utils.StringSliceVal LabelSources *utils.StringSliceVal + EnableSpiffe *bool } type nfdWorker struct { @@ -539,6 +541,9 @@ func (w *nfdWorker) configure(filepath string, overrides string) error { if w.args.Overrides.LabelSources != nil { c.Core.LabelSources = *w.args.Overrides.LabelSources } + if w.args.Overrides.EnableSpiffe != nil { + c.Core.EnableSpiffe = *w.args.Overrides.EnableSpiffe + } c.Core.sanitize() @@ -712,14 +717,15 @@ func (m *nfdWorker) updateNodeFeatureObject(labels Labels) error { }, } - signature, err := m.spiffeClient.SignData(nfr.Spec, spiffe.GetSpiffeId(utils.NodeName())) - klog.InfoS("data signature", "signature", string(signature)) - if err != nil { - klog.ErrorS(err, "error while getting data signature") - return fmt.Errorf("failed to sign CRD data using Spiffe: %w", err) + // If Spiffe is enabled, we add the signature to the annotations section + if m.config.Core.EnableSpiffe { + signature, err := m.spiffeClient.SignData(nfr.Spec, spiffe.GetSpiffeId(utils.NodeName())) + if err != nil { + return fmt.Errorf("failed to sign CRD data using Spiffe: %w", err) + } + encodedSignature := b64.StdEncoding.EncodeToString(signature) + nfr.ObjectMeta.Annotations["signature"] = encodedSignature } - encodedSignature := b64.StdEncoding.EncodeToString(signature) - nfr.ObjectMeta.Annotations["signature"] = encodedSignature nfrCreated, err := cli.NfdV1alpha1().NodeFeatures(namespace).Create(context.TODO(), nfr, metav1.CreateOptions{}) if err != nil { @@ -738,15 +744,16 @@ func (m *nfdWorker) updateNodeFeatureObject(labels Labels) error { Labels: labels, } - signature, err := m.spiffeClient.SignData(nfrUpdated.Spec, spiffe.GetSpiffeId(utils.NodeName())) - encodedSignature := b64.StdEncoding.EncodeToString(signature) - klog.InfoS("data signature", "signature", encodedSignature) - if err != nil { - klog.ErrorS(err, "error while getting data signature") - return fmt.Errorf("failed to sign CRD data using Spiffe: %w", err) - } + if m.config.Core.EnableSpiffe { + signature, err := m.spiffeClient.SignData(nfrUpdated.Spec, spiffe.GetSpiffeId(utils.NodeName())) - nfrUpdated.ObjectMeta.Annotations["signature"] = encodedSignature + if err != nil { + return fmt.Errorf("failed to sign CRD data using Spiffe: %w", err) + } + + encodedSignature := b64.StdEncoding.EncodeToString(signature) + nfrUpdated.ObjectMeta.Annotations["signature"] = encodedSignature + } if !apiequality.Semantic.DeepEqual(nfr, nfrUpdated) { klog.InfoS("updating NodeFeature object", "nodefeature", klog.KObj(nfr))