Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feat integrate spiffe #1663

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions cmd/nfd-master/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,8 @@ func main() {
args.Overrides.ResyncPeriod = overrides.ResyncPeriod
case "nfd-api-parallelism":
args.Overrides.NfdApiParallelism = overrides.NfdApiParallelism
case "enable-spiffe":
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@marquiz should we define this as a feature-gate???

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably not. I think this will never be enabled by default so feature gate feels superfluous (and unintuitive). It would serve as a unnecessary second-level gate (we'd want the enable/disable setting anyway), i.e. you'd need to specify -feature-gates spiffe=true -enable-spiffe

args.Overrides.EnableSpiffe = overrides.EnableSpiffe
}
})

Expand Down Expand Up @@ -145,6 +147,8 @@ func initFlags(flagset *flag.FlagSet) (*master.Args, *master.ConfigOverrideArgs)
flagset.Var(overrides.ResyncPeriod, "resync-period", "Specify the NFD API controller resync period.")
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.")
overrides.EnableSpiffe = flagset.Bool("enable-spiffe", false,
"Enables the Spiffe signature verification of created CRDs. This is still an EXPERIMENTAL feature.")

return args, overrides
}
4 changes: 4 additions & 0 deletions cmd/nfd-worker/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,8 @@ func parseArgs(flags *flag.FlagSet, osArgs ...string) *worker.Args {
args.Overrides.LabelSources = overrides.LabelSources
case "no-owner-refs":
args.Overrides.NoOwnerRefs = overrides.NoOwnerRefs
case "enable-spiffe":
args.Overrides.EnableSpiffe = overrides.EnableSpiffe
}
})

Expand Down Expand Up @@ -136,6 +138,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
}
6 changes: 6 additions & 0 deletions deployment/helm/node-feature-discovery/Chart.lock
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
dependencies:
- name: spire
repository: https://spiffe.github.io/helm-charts-hardened/
version: 0.24.1
digest: sha256:f3b4dc973a59682bf3aa5ca9b53322f57935dd093081e82a37b8082e00becbe9
generated: "2024-12-20T16:52:40.180416+01:00"
4 changes: 4 additions & 0 deletions deployment/helm/node-feature-discovery/Chart.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -13,3 +13,7 @@ keywords:
- node-labels
type: application
version: 0.2.1
dependencies:
- name: spire
version: 0.24.1
repository: https://spiffe.github.io/helm-charts-hardened/
Binary file not shown.
14 changes: 14 additions & 0 deletions deployment/helm/node-feature-discovery/templates/master.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -144,11 +144,25 @@ spec:
{{- with .Values.master.extraArgs }}
{{- toYaml . | nindent 12 }}
{{- end }}
{{- if .Values.spire.enabled }}
- "-enable-spiffe"
{{- end }}
volumeMounts:
{{- if .Values.spire.enabled }}
- name: spire-agent-socket
mountPath: /run/spire/agent-sockets
readOnly: true
{{- end }}
- name: nfd-master-conf
mountPath: "/etc/kubernetes/node-feature-discovery"
readOnly: true
volumes:
{{- if .Values.spire.enabled }}
- name: spire-agent-socket
hostPath:
path: /run/spire/agent-sockets
type: Directory
{{- end }}
- name: nfd-master-conf
configMap:
name: {{ include "node-feature-discovery.fullname" . }}-master-conf
Expand Down
14 changes: 14 additions & 0 deletions deployment/helm/node-feature-discovery/templates/worker.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -109,12 +109,20 @@ spec:
{{- with .Values.gc.extraArgs }}
{{- toYaml . | nindent 8 }}
{{- end }}
{{- if .Values.spire.enabled }}
- "-enable-spiffe"
{{- end }}
ports:
- containerPort: {{ .Values.worker.metricsPort | default "8081"}}
name: metrics
- containerPort: {{ .Values.worker.healthPort | default "8082" }}
name: health
volumeMounts:
{{- if .Values.spire.enabled }}
- name: spire-agent-socket
mountPath: /run/spire/agent-sockets
readOnly: true
{{- end }}
- name: host-boot
mountPath: "/host-boot"
readOnly: true
Expand Down Expand Up @@ -145,6 +153,12 @@ spec:
mountPath: "/etc/kubernetes/node-feature-discovery"
readOnly: true
volumes:
{{- if .Values.spire.enabled }}
- name: spire-agent-socket
hostPath:
path: /run/spire/agent-sockets
type: Directory
{{- end }}
- name: host-boot
hostPath:
path: "/boot"
Expand Down
44 changes: 44 additions & 0 deletions deployment/helm/node-feature-discovery/values.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -597,3 +597,47 @@ prometheus:
enable: false
scrapeInterval: 10s
labels: {}

spire:
enabled: true
global:
spire:
clusterName: "nfd"
trustDomain: "nfd.io"
spire-agent:
kubeletConnectByHostname: "true"
workloadAttestors:
unix:
enabled: true
spire-server:
controllerManager:
enabled: true
identities:
clusterStaticEntries:
node:
parentID: spiffe://nfd.io/spire/server
spiffeID: spiffe://nfd.io/root
selectors:
- k8s_psat:agent_ns:nfd
- k8s_psat:agent_sa:nfd-agent
- k8s_psat:cluster:nfd
nfd:
parentID: spiffe://nfd.io/root
spiffeID: spiffe://nfd.io/worker
selectors:
- k8s:pod-label:app.kubernetes.io/name:node-feature-discovery


caSubject:
commonName: "nfd.io"
country: "US"
organization: "SPIFFE"

upstream:
enabled: false
spiffe-csi-driver:
enabled: false
spiffe-oidc-discovery-provider:
enabled: false
tornjak-frontend:
enabled: false
3 changes: 3 additions & 0 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ require (
github.com/prometheus/client_golang v1.19.1
github.com/smartystreets/goconvey v1.8.1
github.com/spf13/cobra v1.8.1
github.com/spiffe/go-spiffe/v2 v2.4.0
github.com/stretchr/testify v1.10.0
github.com/vektra/errors v0.0.0-20140903201135-c64d83aba85a
golang.org/x/exp v0.0.0-20240719175910-8a7402abbf56
Expand Down Expand Up @@ -68,6 +69,7 @@ require (
github.com/euank/go-kmsg-parser v2.0.0+incompatible // indirect
github.com/felixge/httpsnoop v1.0.4 // indirect
github.com/fxamacker/cbor/v2 v2.7.0 // indirect
github.com/go-jose/go-jose/v4 v4.0.4 // indirect
github.com/go-logr/logr v1.4.2 // indirect
github.com/go-logr/stdr v1.2.2 // indirect
github.com/go-ole/go-ole v1.2.6 // indirect
Expand Down Expand Up @@ -120,6 +122,7 @@ require (
github.com/stoewer/go-strcase v1.3.0 // indirect
github.com/stretchr/objx v0.5.2 // indirect
github.com/x448/float16 v0.8.4 // indirect
github.com/zeebo/errs v1.3.0 // indirect
go.etcd.io/etcd/api/v3 v3.5.16 // indirect
go.etcd.io/etcd/client/pkg/v3 v3.5.16 // indirect
go.etcd.io/etcd/client/v3 v3.5.16 // indirect
Expand Down
6 changes: 6 additions & 0 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,8 @@ github.com/fsnotify/fsnotify v1.8.0 h1:dAwr6QBTBZIkG8roQaJjGof0pp0EeF+tNV7YBP3F/
github.com/fsnotify/fsnotify v1.8.0/go.mod h1:8jBTzvmWwFyi3Pb8djgCCO5IBqzKJ/Jwo8TRcHyHii0=
github.com/fxamacker/cbor/v2 v2.7.0 h1:iM5WgngdRBanHcxugY4JySA0nk1wZorNOpTgCMedv5E=
github.com/fxamacker/cbor/v2 v2.7.0/go.mod h1:pxXPTn3joSm21Gbwsv0w9OSA2y1HFR9qXEeXQVeNoDQ=
github.com/go-jose/go-jose/v4 v4.0.4 h1:VsjPI33J0SB9vQM6PLmNjoHqMQNGPiZ0rHL7Ni7Q6/E=
github.com/go-jose/go-jose/v4 v4.0.4/go.mod h1:NKb5HO1EZccyMpiZNbdUw/14tiXNyUJh188dfnMCAfc=
github.com/go-logr/logr v1.2.2/go.mod h1:jdQByPbusPIv2/zmleS9BjJVeZ6kBagPoEUsqbVz/1A=
github.com/go-logr/logr v1.4.2 h1:6pFjapn8bFcIbiKo3XT4j/BhANplGihG6tvd+8rYgrY=
github.com/go-logr/logr v1.4.2/go.mod h1:9T104GzyrTigFIr8wt5mBrctHMim0Nb2HLGrmQ40KvY=
Expand Down Expand Up @@ -228,6 +230,8 @@ github.com/spf13/cobra v1.8.1 h1:e5/vxKd/rZsfSJMUX1agtjeTDf+qv1/JdBF8gg5k9ZM=
github.com/spf13/cobra v1.8.1/go.mod h1:wHxEcudfqmLYa8iTfL+OuZPbBZkmvliBWKIezN3kD9Y=
github.com/spf13/pflag v1.0.5 h1:iy+VFUOCP1a+8yFto/drg2CJ5u0yRoB7fZw3DKv/JXA=
github.com/spf13/pflag v1.0.5/go.mod h1:McXfInJRrz4CZXVZOBLb0bTZqETkiAhM9Iw0y3An2Bg=
github.com/spiffe/go-spiffe/v2 v2.4.0 h1:j/FynG7hi2azrBG5cvjRcnQ4sux/VNj8FAVc99Fl66c=
github.com/spiffe/go-spiffe/v2 v2.4.0/go.mod h1:m5qJ1hGzjxjtrkGHZupoXHo/FDWwCB1MdSyBzfHugx0=
github.com/stoewer/go-strcase v1.3.0 h1:g0eASXYtp+yvN9fK8sH94oCIk0fau9uV1/ZdJ0AVEzs=
github.com/stoewer/go-strcase v1.3.0/go.mod h1:fAH5hQ5pehh+j3nZfvwdk2RgEgQjAoM8wodgtPmh1xo=
github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME=
Expand All @@ -252,6 +256,8 @@ github.com/xiang90/probing v0.0.0-20221125231312-a49e3df8f510 h1:S2dVYn90KE98chq
github.com/xiang90/probing v0.0.0-20221125231312-a49e3df8f510/go.mod h1:UETIi67q53MR2AWcXfiuqkDkRtnGDLqkBTpCHuJHxtU=
github.com/yuin/goldmark v1.1.27/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74=
github.com/yuin/goldmark v1.2.1/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74=
github.com/zeebo/errs v1.3.0 h1:hmiaKqgYZzcVgRL1Vkc1Mn2914BbzB0IBxs+ebeutGs=
github.com/zeebo/errs v1.3.0/go.mod h1:sgbWHsvVuTPHcqJJGQ1WhI5KbWlHYz+2+2C/LSEtCw4=
go.etcd.io/bbolt v1.3.11 h1:yGEzV1wPz2yVCLsD8ZAiGHhHVlczyC9d1rP43/VCRJ0=
go.etcd.io/bbolt v1.3.11/go.mod h1:dksAq7YMXoljX0xu6VF5DMZGbhYYoLUalEiSySYAS4I=
go.etcd.io/etcd/api/v3 v3.5.16 h1:WvmyJVbjWqK4R1E+B12RRHz3bRGy9XVfh++MgbN+6n0=
Expand Down
60 changes: 58 additions & 2 deletions pkg/nfd-master/nfd-master.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,15 +52,20 @@ import (
"sigs.k8s.io/yaml"

nfdclientset "sigs.k8s.io/node-feature-discovery/api/generated/clientset/versioned"
"sigs.k8s.io/node-feature-discovery/api/nfd/v1alpha1"
nfdv1alpha1 "sigs.k8s.io/node-feature-discovery/api/nfd/v1alpha1"
"sigs.k8s.io/node-feature-discovery/pkg/apis/nfd/nodefeaturerule"
"sigs.k8s.io/node-feature-discovery/pkg/apis/nfd/validate"
nfdfeatures "sigs.k8s.io/node-feature-discovery/pkg/features"
"sigs.k8s.io/node-feature-discovery/pkg/utils"
klogutils "sigs.k8s.io/node-feature-discovery/pkg/utils/klog"
spiffe "sigs.k8s.io/node-feature-discovery/pkg/utils/spiffe"
"sigs.k8s.io/node-feature-discovery/pkg/version"
)

// SocketPath specifies Spiffe Socket Path
const SocketPath = "unix:///run/spire/agent-sockets/api.sock"

// Labels are a Kubernetes representation of discovered features.
type Labels map[string]string

Expand Down Expand Up @@ -93,6 +98,7 @@ type NFDConfig struct {
NfdApiParallelism int
Klog klogutils.KlogConfigOpts
Restrictions Restrictions
EnableSpiffe bool
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, looking at this maybe it would be safest to only have this as a command line argument (i.e. NOT at as a dynamically configurable config file setting). Just to make it very clear if/when the setting is changed. WDYT?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sounds good for me!

}

// LeaderElectionConfig contains the configuration for leader election
Expand All @@ -111,6 +117,7 @@ type ConfigOverrideArgs struct {
NoPublish *bool
ResyncPeriod *utils.DurationVal
NfdApiParallelism *int
EnableSpiffe *bool
}

// Args holds command line arguments
Expand Down Expand Up @@ -158,7 +165,8 @@ type nfdMaster struct {
nfdClient nfdclientset.Interface
updaterPool *updaterPool
deniedNs
config *NFDConfig
config *NFDConfig
spiffeClient *spiffe.SpiffeClient
}

// NewNfdMaster creates a new NfdMaster server instance.
Expand Down Expand Up @@ -216,6 +224,12 @@ func NewNfdMaster(opts ...NfdMasterOption) (NfdMaster, error) {

nfd.updaterPool = newUpdaterPool(nfd)

spiffeClient, err := spiffe.NewSpiffeClient(SocketPath)
if err != nil {
return nfd, err
}
nfd.spiffeClient = spiffeClient

return nfd, nil
}

Expand Down Expand Up @@ -257,14 +271,15 @@ func newDefaultConfig() *NFDConfig {
RetryPeriod: utils.DurationVal{Duration: time.Duration(2) * time.Second},
RenewDeadline: utils.DurationVal{Duration: time.Duration(10) * time.Second},
},
Klog: make(map[string]string),
Restrictions: Restrictions{
DisableLabels: false,
DisableExtendedResources: false,
DisableAnnotations: false,
AllowOverwrite: true,
DenyNodeFeatureLabels: false,
},
Klog: make(map[string]string),
EnableSpiffe: false,
}
}

Expand Down Expand Up @@ -680,6 +695,14 @@ func (m *nfdMaster) getAndMergeNodeFeatures(nodeName string) (*nfdv1alpha1.NodeF
return filteredObjs[i].Namespace < filteredObjs[j].Namespace
})

// If spiffe is enabled, we should filter out the non verified NFD objects
if m.config.EnableSpiffe {
filteredObjs, err = m.getVerifiedNFDObjects(filteredObjs)
if err != nil {
return &nfdv1alpha1.NodeFeature{}, err
}
}

if len(filteredObjs) > 0 {
// Merge in features
//
Expand Down Expand Up @@ -1246,6 +1269,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")
Expand Down Expand Up @@ -1446,3 +1472,33 @@ func patchNode(cli k8sclient.Interface, nodeName string, patches []utils.JsonPat
func patchNodeStatus(cli k8sclient.Interface, nodeName string, patches []utils.JsonPatch) error {
return patchNode(cli, nodeName, patches, "status")
}

func (m *nfdMaster) getVerifiedNFDObjects(objs []*v1alpha1.NodeFeature) ([]*v1alpha1.NodeFeature, error) {
verifiedObjects := []*v1alpha1.NodeFeature{}

workerPrivateKey, workerPublicKey, err := m.spiffeClient.GetWorkerKeys()
if err != nil {
return verifiedObjects, err
}

for _, obj := range objs {
spiffeObj := spiffe.SpiffeObject{
Spec: obj.Spec,
Name: obj.Name,
Namespace: obj.Namespace,
Labels: obj.Labels,
}
isSignatureVerified, err := spiffe.VerifyDataSignature(spiffeObj, obj.Annotations["signature"], workerPrivateKey, workerPublicKey)
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
}
Loading