From c64e103464999c9839e51946d02b02aa31cfce10 Mon Sep 17 00:00:00 2001 From: Michail Resvanis Date: Tue, 27 Sep 2022 18:00:42 +0200 Subject: [PATCH] Add module loader and device plugin SAs when not defined This change adds ServiceAccounts for the module loader and the device plugin workloads of a Module, when the ServiceAccounts are not defined in the Module.Spec.ModuleLoader.ServiceAccountName and Module.Spec.DevicePlugin.ServiceAccountName respectively. This way we avoid using the K8S default ServiceAccount, which may have different permissions that the module loader and/or the device plugin workloads require. Specifically, this change: - adds the required ServiceAccount permissions to the module reconciler - adds the following Module reconciliation steps when the Module.Spec.ModuleLoader.ServiceAccountName is empty: - adds a module loader ServiceAccount - adds the module loaderServiceAccount to the module loader DaemonSet - adds the following Module reconciliation steps when the Module.Spec.DevicePlugin.ServiceAccountName is empty: - adds a device plugin ServiceAccount - adds the device plugin ServiceAccount to the device plugin DaemonSet Signed-off-by: Michail Resvanis --- config/rbac/role.yaml | 11 ++ controllers/module_reconciler.go | 17 ++ controllers/module_reconciler_test.go | 128 +++++++++++++-- internal/daemonset/daemonset.go | 15 +- internal/daemonset/daemonset_test.go | 28 ++++ internal/rbac/mock_rbac.go | 64 ++++++++ internal/rbac/rbac.go | 84 ++++++++++ internal/rbac/rbac_test.go | 218 ++++++++++++++++++++++++++ internal/rbac/suite_test.go | 40 +++++ main.go | 4 +- 10 files changed, 589 insertions(+), 20 deletions(-) create mode 100644 internal/rbac/mock_rbac.go create mode 100644 internal/rbac/rbac.go create mode 100644 internal/rbac/rbac_test.go create mode 100644 internal/rbac/suite_test.go diff --git a/config/rbac/role.yaml b/config/rbac/role.yaml index 36a85d2d3..e37e7bbe3 100644 --- a/config/rbac/role.yaml +++ b/config/rbac/role.yaml @@ -51,6 +51,17 @@ rules: - get - list - watch +- apiGroups: + - "" + resources: + - serviceaccounts + verbs: + - create + - delete + - get + - list + - patch + - watch - apiGroups: - kmm.sigs.k8s.io resources: diff --git a/controllers/module_reconciler.go b/controllers/module_reconciler.go index 21a208ac0..1047a6ae3 100644 --- a/controllers/module_reconciler.go +++ b/controllers/module_reconciler.go @@ -28,6 +28,7 @@ import ( "github.com/kubernetes-sigs/kernel-module-management/internal/filter" "github.com/kubernetes-sigs/kernel-module-management/internal/metrics" "github.com/kubernetes-sigs/kernel-module-management/internal/module" + "github.com/kubernetes-sigs/kernel-module-management/internal/rbac" "github.com/kubernetes-sigs/kernel-module-management/internal/registry" "github.com/kubernetes-sigs/kernel-module-management/internal/statusupdater" appsv1 "k8s.io/api/apps/v1" @@ -52,6 +53,7 @@ type ModuleReconciler struct { client.Client buildAPI build.Manager + rbacAPI rbac.RBACCreator daemonAPI daemonset.DaemonSetCreator kernelAPI module.KernelMapper metricsAPI metrics.Metrics @@ -63,6 +65,7 @@ type ModuleReconciler struct { func NewModuleReconciler( client client.Client, buildAPI build.Manager, + rbacAPI rbac.RBACCreator, daemonAPI daemonset.DaemonSetCreator, kernelAPI module.KernelMapper, metricsAPI metrics.Metrics, @@ -72,6 +75,7 @@ func NewModuleReconciler( return &ModuleReconciler{ Client: client, buildAPI: buildAPI, + rbacAPI: rbacAPI, daemonAPI: daemonAPI, kernelAPI: kernelAPI, metricsAPI: metricsAPI, @@ -87,6 +91,7 @@ func NewModuleReconciler( //+kubebuilder:rbac:groups=apps,resources=daemonsets,verbs=create;delete;get;list;patch;watch //+kubebuilder:rbac:groups="core",resources=nodes,verbs=get;list;watch //+kubebuilder:rbac:groups="core",resources=secrets,verbs=get;list;watch +//+kubebuilder:rbac:groups="core",resources=serviceaccounts,verbs=create;delete;get;list;patch;watch //+kubebuilder:rbac:groups="batch",resources=jobs,verbs=create;list;watch;delete // Reconcile lists all nodes and looks for kernels that match its mappings. @@ -109,6 +114,17 @@ func (r *ModuleReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr r.setKMMOMetrics(ctx) + if mod.Spec.ModuleLoader.ServiceAccountName == "" { + if err := r.rbacAPI.CreateModuleLoaderServiceAccount(ctx, *mod); err != nil { + return res, fmt.Errorf("could not create module-loader's ServiceAccount: %w", err) + } + } + if mod.Spec.DevicePlugin != nil && mod.Spec.DevicePlugin.ServiceAccountName == "" { + if err := r.rbacAPI.CreateDevicePluginServiceAccount(ctx, *mod); err != nil { + return res, fmt.Errorf("could not create device-plugin's ServiceAccount: %w", err) + } + } + targetedNodes, err := r.getNodesListBySelector(ctx, mod) if err != nil { return res, fmt.Errorf("could get targeted nodes for module %s: %w", mod.Name, err) @@ -359,6 +375,7 @@ func (r *ModuleReconciler) SetupWithManager(mgr ctrl.Manager, kernelLabel string return ctrl.NewControllerManagedBy(mgr). For(&kmmv1beta1.Module{}). Owns(&appsv1.DaemonSet{}). + Owns(&v1.ServiceAccount{}). Owns(&batchv1.Job{}). Watches( &source.Kind{Type: &v1.Node{}}, diff --git a/controllers/module_reconciler_test.go b/controllers/module_reconciler_test.go index 9f54c951a..610bcbdee 100644 --- a/controllers/module_reconciler_test.go +++ b/controllers/module_reconciler_test.go @@ -11,6 +11,7 @@ import ( "github.com/kubernetes-sigs/kernel-module-management/internal/daemonset" "github.com/kubernetes-sigs/kernel-module-management/internal/metrics" "github.com/kubernetes-sigs/kernel-module-management/internal/module" + "github.com/kubernetes-sigs/kernel-module-management/internal/rbac" "github.com/kubernetes-sigs/kernel-module-management/internal/registry" "github.com/kubernetes-sigs/kernel-module-management/internal/statusupdater" . "github.com/onsi/ginkgo/v2" @@ -34,6 +35,7 @@ var _ = Describe("ModuleReconciler_Reconcile", func() { ctrl *gomock.Controller clnt *client.MockClient mockBM *build.MockManager + mockRC *rbac.MockRBACCreator mockDC *daemonset.MockDaemonSetCreator mockKM *module.MockKernelMapper mockMetrics *metrics.MockMetrics @@ -45,6 +47,7 @@ var _ = Describe("ModuleReconciler_Reconcile", func() { ctrl = gomock.NewController(GinkgoT()) clnt = client.NewMockClient(ctrl) mockBM = build.NewMockManager(ctrl) + mockRC = rbac.NewMockRBACCreator(ctrl) mockDC = daemonset.NewMockDaemonSetCreator(ctrl) mockKM = module.NewMockKernelMapper(ctrl) mockMetrics = metrics.NewMockMetrics(ctrl) @@ -71,7 +74,7 @@ var _ = Describe("ModuleReconciler_Reconcile", func() { apierrors.NewNotFound(schema.GroupResource{}, moduleName), ) - mr := NewModuleReconciler(clnt, mockBM, mockDC, mockKM, mockMetrics, nil, mockRegistry, mockSU) + mr := NewModuleReconciler(clnt, mockBM, mockRC, mockDC, mockKM, mockMetrics, nil, mockRegistry, mockSU) Expect( mr.Reconcile(ctx, req), ).To( @@ -79,7 +82,77 @@ var _ = Describe("ModuleReconciler_Reconcile", func() { ) }) + It("should add the module loader and device plugin ServiceAccounts if they are not set", func() { + mod := kmmv1beta1.Module{ + ObjectMeta: metav1.ObjectMeta{ + Name: moduleName, + Namespace: namespace, + }, + Spec: kmmv1beta1.ModuleSpec{ + Selector: map[string]string{"key": "value"}, + ModuleLoader: kmmv1beta1.ModuleLoaderSpec{ + ServiceAccountName: "", + }, + DevicePlugin: &kmmv1beta1.DevicePluginSpec{ + ServiceAccountName: "", + }, + }, + } + ds := appsv1.DaemonSet{ + ObjectMeta: metav1.ObjectMeta{ + Name: moduleName + "-device-plugin", + Namespace: namespace, + }, + } + + gomock.InOrder( + clnt.EXPECT().Get(ctx, req.NamespacedName, gomock.Any()).DoAndReturn( + func(_ interface{}, _ interface{}, m *kmmv1beta1.Module) error { + m.ObjectMeta = mod.ObjectMeta + m.Spec = mod.Spec + return nil + }, + ), + clnt.EXPECT().List(ctx, gomock.Any(), gomock.Any()).DoAndReturn( + func(_ interface{}, list *kmmv1beta1.ModuleList, _ ...interface{}) error { + list.Items = []kmmv1beta1.Module{mod} + return nil + }, + ), + mockMetrics.EXPECT().SetExistingKMMOModules(1), + mockRC.EXPECT().CreateModuleLoaderServiceAccount(ctx, gomock.Any()).Return(nil), + mockRC.EXPECT().CreateDevicePluginServiceAccount(ctx, gomock.Any()).Return(nil), + clnt.EXPECT().List(ctx, gomock.Any(), gomock.Any()).DoAndReturn( + func(_ interface{}, list *v1.NodeList, _ ...interface{}) error { + list.Items = []v1.Node{} + return nil + }, + ), + clnt.EXPECT().Get(ctx, gomock.Any(), gomock.Any()).Return(apierrors.NewNotFound(schema.GroupResource{}, "whatever")), + clnt.EXPECT().Get(ctx, gomock.Any(), gomock.Any()).Return(apierrors.NewNotFound(schema.GroupResource{}, "whatever")), + mockDC.EXPECT().SetDevicePluginAsDesired(context.Background(), &ds, gomock.AssignableToTypeOf(&mod)), + clnt.EXPECT().Create(ctx, gomock.Any()).Return(nil), + mockMetrics.EXPECT().SetCompletedStage(moduleName, namespace, "", metrics.DevicePluginStage, false), + ) + + mr := NewModuleReconciler(clnt, mockBM, mockRC, mockDC, mockKM, mockMetrics, nil, mockRegistry, mockSU) + + dsByKernelVersion := make(map[string]*appsv1.DaemonSet) + + gomock.InOrder( + mockDC.EXPECT().ModuleDaemonSetsByKernelVersion(ctx, moduleName, namespace).Return(dsByKernelVersion, nil), + mockDC.EXPECT().GarbageCollect(ctx, dsByKernelVersion, sets.NewString()), + mockSU.EXPECT().ModuleUpdateStatus(ctx, &mod, []v1.Node{}, []v1.Node{}, dsByKernelVersion).Return(nil), + ) + + res, err := mr.Reconcile(context.Background(), req) + Expect(err).NotTo(HaveOccurred()) + Expect(res).To(Equal(reconcile.Result{})) + }) + It("should do nothing when no nodes match the selector", func() { + const serviceAccountName = "module-loader-service-account" + mod := kmmv1beta1.Module{ ObjectMeta: metav1.ObjectMeta{ Name: moduleName, @@ -87,6 +160,9 @@ var _ = Describe("ModuleReconciler_Reconcile", func() { }, Spec: kmmv1beta1.ModuleSpec{ Selector: map[string]string{"key": "value"}, + ModuleLoader: kmmv1beta1.ModuleLoaderSpec{ + ServiceAccountName: serviceAccountName, + }, }, } @@ -113,7 +189,7 @@ var _ = Describe("ModuleReconciler_Reconcile", func() { ), ) - mr := NewModuleReconciler(clnt, mockBM, mockDC, mockKM, mockMetrics, nil, mockRegistry, mockSU) + mr := NewModuleReconciler(clnt, mockBM, mockRC, mockDC, mockKM, mockMetrics, nil, mockRegistry, mockSU) dsByKernelVersion := make(map[string]*appsv1.DaemonSet) @@ -129,7 +205,10 @@ var _ = Describe("ModuleReconciler_Reconcile", func() { }) It("should remove obsolete DaemonSets when no nodes match the selector", func() { - const kernelVersion = "1.2.3" + const ( + kernelVersion = "1.2.3" + serviceAccountName = "module-loader-service-account" + ) mod := kmmv1beta1.Module{ ObjectMeta: metav1.ObjectMeta{ @@ -138,6 +217,9 @@ var _ = Describe("ModuleReconciler_Reconcile", func() { }, Spec: kmmv1beta1.ModuleSpec{ Selector: map[string]string{"key": "value"}, + ModuleLoader: kmmv1beta1.ModuleLoaderSpec{ + ServiceAccountName: serviceAccountName, + }, }, } @@ -171,7 +253,7 @@ var _ = Describe("ModuleReconciler_Reconcile", func() { ), ) - mr := NewModuleReconciler(clnt, mockBM, mockDC, mockKM, mockMetrics, nil, mockRegistry, mockSU) + mr := NewModuleReconciler(clnt, mockBM, mockRC, mockDC, mockKM, mockMetrics, nil, mockRegistry, mockSU) dsByKernelVersion := map[string]*appsv1.DaemonSet{kernelVersion: &ds} @@ -188,8 +270,9 @@ var _ = Describe("ModuleReconciler_Reconcile", func() { It("should create a DaemonSet when a node matches the selector", func() { const ( - imageName = "test-image" - kernelVersion = "1.2.3" + imageName = "test-image" + kernelVersion = "1.2.3" + serviceAccountName = "module-loader-service-account" ) mappings := []kmmv1beta1.KernelMapping{ @@ -208,6 +291,7 @@ var _ = Describe("ModuleReconciler_Reconcile", func() { }, Spec: kmmv1beta1.ModuleSpec{ ModuleLoader: kmmv1beta1.ModuleLoaderSpec{ + ServiceAccountName: serviceAccountName, Container: kmmv1beta1.ModuleLoaderContainerSpec{ KernelMappings: mappings, }, @@ -232,7 +316,7 @@ var _ = Describe("ModuleReconciler_Reconcile", func() { dsByKernelVersion := make(map[string]*appsv1.DaemonSet) - mr := NewModuleReconciler(clnt, mockBM, mockDC, mockKM, mockMetrics, nil, mockRegistry, mockSU) + mr := NewModuleReconciler(clnt, mockBM, mockRC, mockDC, mockKM, mockMetrics, nil, mockRegistry, mockSU) ds := appsv1.DaemonSet{ ObjectMeta: metav1.ObjectMeta{ @@ -280,8 +364,9 @@ var _ = Describe("ModuleReconciler_Reconcile", func() { It("should patch the DaemonSet when it already exists", func() { const ( - imageName = "test-image" - kernelVersion = "1.2.3" + imageName = "test-image" + kernelVersion = "1.2.3" + serviceAccountName = "module-loader-service-account" ) osConfig := module.NodeOSConfig{} @@ -302,6 +387,7 @@ var _ = Describe("ModuleReconciler_Reconcile", func() { }, Spec: kmmv1beta1.ModuleSpec{ ModuleLoader: kmmv1beta1.ModuleLoaderSpec{ + ServiceAccountName: serviceAccountName, Container: kmmv1beta1.ModuleLoaderContainerSpec{ KernelMappings: mappings, }, @@ -361,7 +447,7 @@ var _ = Describe("ModuleReconciler_Reconcile", func() { clnt.EXPECT().Patch(ctx, gomock.Any(), gomock.Any()), ) - mr := NewModuleReconciler(clnt, mockBM, mockDC, mockKM, mockMetrics, nil, mockRegistry, mockSU) + mr := NewModuleReconciler(clnt, mockBM, mockRC, mockDC, mockKM, mockMetrics, nil, mockRegistry, mockSU) dsByKernelVersion := map[string]*appsv1.DaemonSet{kernelVersion: &ds} @@ -387,6 +473,9 @@ var _ = Describe("ModuleReconciler_Reconcile", func() { const ( imageName = "test-image" kernelVersion = "1.2.3" + + moduleLoaderServiceAccountName = "module-loader-service-account" + devicePluginServiceAccountName = "device-plugin-service-account" ) mappings := []kmmv1beta1.KernelMapping{ @@ -402,8 +491,11 @@ var _ = Describe("ModuleReconciler_Reconcile", func() { Namespace: namespace, }, Spec: kmmv1beta1.ModuleSpec{ - DevicePlugin: &kmmv1beta1.DevicePluginSpec{}, + DevicePlugin: &kmmv1beta1.DevicePluginSpec{ + ServiceAccountName: devicePluginServiceAccountName, + }, ModuleLoader: kmmv1beta1.ModuleLoaderSpec{ + ServiceAccountName: moduleLoaderServiceAccountName, Container: kmmv1beta1.ModuleLoaderContainerSpec{ KernelMappings: mappings, }, @@ -412,7 +504,7 @@ var _ = Describe("ModuleReconciler_Reconcile", func() { }, } - mr := NewModuleReconciler(clnt, mockBM, mockDC, mockKM, mockMetrics, nil, mockRegistry, mockSU) + mr := NewModuleReconciler(clnt, mockBM, mockRC, mockDC, mockKM, mockMetrics, nil, mockRegistry, mockSU) ds := appsv1.DaemonSet{ ObjectMeta: metav1.ObjectMeta{ @@ -462,6 +554,7 @@ var _ = Describe("ModuleReconciler_handleBuild", func() { ctrl *gomock.Controller clnt *client.MockClient mockBM *build.MockManager + mockRC *rbac.MockRBACCreator mockDC *daemonset.MockDaemonSetCreator mockKM *module.MockKernelMapper mockMetrics *metrics.MockMetrics @@ -473,6 +566,7 @@ var _ = Describe("ModuleReconciler_handleBuild", func() { ctrl = gomock.NewController(GinkgoT()) clnt = client.NewMockClient(ctrl) mockBM = build.NewMockManager(ctrl) + mockRC = rbac.NewMockRBACCreator(ctrl) mockDC = daemonset.NewMockDaemonSetCreator(ctrl) mockKM = module.NewMockKernelMapper(ctrl) mockMetrics = metrics.NewMockMetrics(ctrl) @@ -494,7 +588,7 @@ var _ = Describe("ModuleReconciler_handleBuild", func() { mod := &kmmv1beta1.Module{} - mr := NewModuleReconciler(clnt, mockBM, mockDC, mockKM, mockMetrics, nil, mockRegistry, mockSU) + mr := NewModuleReconciler(clnt, mockBM, mockRC, mockDC, mockKM, mockMetrics, nil, mockRegistry, mockSU) res, err := mr.handleBuild(context.Background(), mod, km, kernelVersion) Expect(err).NotTo(HaveOccurred()) @@ -524,7 +618,7 @@ var _ = Describe("ModuleReconciler_handleBuild", func() { }, ) - mr := NewModuleReconciler(clnt, mockBM, mockDC, mockKM, mockMetrics, nil, mockRegistry, mockSU) + mr := NewModuleReconciler(clnt, mockBM, mockRC, mockDC, mockKM, mockMetrics, nil, mockRegistry, mockSU) res, err := mr.handleBuild(context.Background(), mod, km, kernelVersion) Expect(err).NotTo(HaveOccurred()) Expect(res).To(BeFalse()) @@ -550,7 +644,7 @@ var _ = Describe("ModuleReconciler_handleBuild", func() { mockMetrics.EXPECT().SetCompletedStage(mod.Name, mod.Namespace, kernelVersion, metrics.BuildStage, false), ) - mr := NewModuleReconciler(clnt, mockBM, mockDC, mockKM, mockMetrics, nil, mockRegistry, mockSU) + mr := NewModuleReconciler(clnt, mockBM, mockRC, mockDC, mockKM, mockMetrics, nil, mockRegistry, mockSU) res, err := mr.handleBuild(context.Background(), mod, km, kernelVersion) Expect(err).NotTo(HaveOccurred()) Expect(res).To(BeTrue()) @@ -576,7 +670,7 @@ var _ = Describe("ModuleReconciler_handleBuild", func() { mockMetrics.EXPECT().SetCompletedStage(mod.Name, mod.Namespace, kernelVersion, metrics.BuildStage, false), ) - mr := NewModuleReconciler(clnt, mockBM, mockDC, mockKM, mockMetrics, nil, mockRegistry, mockSU) + mr := NewModuleReconciler(clnt, mockBM, mockRC, mockDC, mockKM, mockMetrics, nil, mockRegistry, mockSU) res, err := mr.handleBuild(context.Background(), mod, km, kernelVersion) Expect(err).NotTo(HaveOccurred()) Expect(res).To(BeTrue()) @@ -602,7 +696,7 @@ var _ = Describe("ModuleReconciler_handleBuild", func() { mockMetrics.EXPECT().SetCompletedStage(mod.Name, mod.Namespace, kernelVersion, metrics.BuildStage, true), ) - mr := NewModuleReconciler(clnt, mockBM, mockDC, mockKM, mockMetrics, nil, mockRegistry, mockSU) + mr := NewModuleReconciler(clnt, mockBM, mockRC, mockDC, mockKM, mockMetrics, nil, mockRegistry, mockSU) res, err := mr.handleBuild(context.Background(), mod, km, kernelVersion) Expect(err).NotTo(HaveOccurred()) Expect(res).To(BeFalse()) diff --git a/internal/daemonset/daemonset.go b/internal/daemonset/daemonset.go index 85f1dcc75..1b23d65a6 100644 --- a/internal/daemonset/daemonset.go +++ b/internal/daemonset/daemonset.go @@ -8,6 +8,7 @@ import ( kmmv1beta1 "github.com/kubernetes-sigs/kernel-module-management/api/v1beta1" "github.com/kubernetes-sigs/kernel-module-management/internal/constants" + "github.com/kubernetes-sigs/kernel-module-management/internal/rbac" appsv1 "k8s.io/api/apps/v1" v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -190,6 +191,11 @@ func (dc *daemonSetGenerator) SetDriverContainerAsDesired(ctx context.Context, d container.VolumeMounts = append(container.VolumeMounts, firmwareVolumeMount) } + serviceAccountName := mod.Spec.ModuleLoader.ServiceAccountName + if serviceAccountName == "" { + serviceAccountName = rbac.GenerateModuleLoaderServiceAccountName(mod) + } + ds.Spec = appsv1.DaemonSetSpec{ Template: v1.PodTemplateSpec{ ObjectMeta: metav1.ObjectMeta{ @@ -201,7 +207,7 @@ func (dc *daemonSetGenerator) SetDriverContainerAsDesired(ctx context.Context, d ImagePullSecrets: GetPodPullSecrets(mod.Spec.ImageRepoSecret), NodeSelector: nodeSelector, PriorityClassName: "system-node-critical", - ServiceAccountName: mod.Spec.ModuleLoader.ServiceAccountName, + ServiceAccountName: serviceAccountName, Volumes: volumes, }, }, @@ -248,6 +254,11 @@ func (dc *daemonSetGenerator) SetDevicePluginAsDesired(ctx context.Context, ds * OverrideLabels(ds.GetLabels(), standardLabels), ) + serviceAccountName := mod.Spec.DevicePlugin.ServiceAccountName + if serviceAccountName == "" { + serviceAccountName = rbac.GenerateDevicePluginServiceAccountName(*mod) + } + ds.Spec = appsv1.DaemonSetSpec{ Selector: &metav1.LabelSelector{MatchLabels: standardLabels}, Template: v1.PodTemplateSpec{ @@ -272,7 +283,7 @@ func (dc *daemonSetGenerator) SetDevicePluginAsDesired(ctx context.Context, ds * PriorityClassName: "system-node-critical", ImagePullSecrets: GetPodPullSecrets(mod.Spec.ImageRepoSecret), NodeSelector: map[string]string{getDriverContainerNodeLabel(mod.Name): ""}, - ServiceAccountName: mod.Spec.DevicePlugin.ServiceAccountName, + ServiceAccountName: serviceAccountName, Volumes: append([]v1.Volume{devicePluginVolume}, mod.Spec.DevicePlugin.Volumes...), }, }, diff --git a/internal/daemonset/daemonset_test.go b/internal/daemonset/daemonset_test.go index f62258c39..0ee1371fb 100644 --- a/internal/daemonset/daemonset_test.go +++ b/internal/daemonset/daemonset_test.go @@ -120,7 +120,20 @@ var _ = Describe("SetDriverContainerAsDesired", func() { Expect(ds.Spec.Template.Spec.Volumes[1]).To(Equal(vol)) Expect(ds.Spec.Template.Spec.Containers[0].VolumeMounts).To(HaveLen(2)) Expect(ds.Spec.Template.Spec.Containers[0].VolumeMounts[1]).To(Equal(volm)) + }) + It("should add the default ServiceAccount to the module loader if it is not set in the spec", func() { + mod := kmmv1beta1.Module{ + Spec: kmmv1beta1.ModuleSpec{ + Selector: map[string]string{"has-feature-x": "true"}, + }, + } + + ds := appsv1.DaemonSet{} + + err := dg.SetDriverContainerAsDesired(context.Background(), &ds, "test-image", mod, kernelVersion) + Expect(err).NotTo(HaveOccurred()) + Expect(ds.Spec.Template.Spec.ServiceAccountName).To(Equal(mod.Name + "-module-loader")) }) It("should work as expected", func() { @@ -385,6 +398,21 @@ var _ = Describe("SetDevicePluginAsDesired", func() { Expect(ds.Spec.Template.Spec.Volumes[1]).To(Equal(vol)) }) + It("should add the default ServiceAccount to the device plugin if it is not set in the spec", func() { + mod := kmmv1beta1.Module{ + Spec: kmmv1beta1.ModuleSpec{ + Selector: map[string]string{"has-feature-x": "true"}, + DevicePlugin: &kmmv1beta1.DevicePluginSpec{}, + }, + } + + ds := appsv1.DaemonSet{} + + err := dg.SetDevicePluginAsDesired(context.Background(), &ds, &mod) + Expect(err).NotTo(HaveOccurred()) + Expect(ds.Spec.Template.Spec.ServiceAccountName).To(Equal(mod.Name + "-device-plugin")) + }) + It("should work as expected", func() { const ( dsName = "ds-name" diff --git a/internal/rbac/mock_rbac.go b/internal/rbac/mock_rbac.go new file mode 100644 index 000000000..b71a8e241 --- /dev/null +++ b/internal/rbac/mock_rbac.go @@ -0,0 +1,64 @@ +// Code generated by MockGen. DO NOT EDIT. +// Source: rbac.go + +// Package rbac is a generated GoMock package. +package rbac + +import ( + context "context" + reflect "reflect" + + gomock "github.com/golang/mock/gomock" + v1beta1 "github.com/kubernetes-sigs/kernel-module-management/api/v1beta1" +) + +// MockRBACCreator is a mock of RBACCreator interface. +type MockRBACCreator struct { + ctrl *gomock.Controller + recorder *MockRBACCreatorMockRecorder +} + +// MockRBACCreatorMockRecorder is the mock recorder for MockRBACCreator. +type MockRBACCreatorMockRecorder struct { + mock *MockRBACCreator +} + +// NewMockRBACCreator creates a new mock instance. +func NewMockRBACCreator(ctrl *gomock.Controller) *MockRBACCreator { + mock := &MockRBACCreator{ctrl: ctrl} + mock.recorder = &MockRBACCreatorMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use. +func (m *MockRBACCreator) EXPECT() *MockRBACCreatorMockRecorder { + return m.recorder +} + +// CreateDevicePluginServiceAccount mocks base method. +func (m *MockRBACCreator) CreateDevicePluginServiceAccount(ctx context.Context, mod v1beta1.Module) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "CreateDevicePluginServiceAccount", ctx, mod) + ret0, _ := ret[0].(error) + return ret0 +} + +// CreateDevicePluginServiceAccount indicates an expected call of CreateDevicePluginServiceAccount. +func (mr *MockRBACCreatorMockRecorder) CreateDevicePluginServiceAccount(ctx, mod interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CreateDevicePluginServiceAccount", reflect.TypeOf((*MockRBACCreator)(nil).CreateDevicePluginServiceAccount), ctx, mod) +} + +// CreateModuleLoaderServiceAccount mocks base method. +func (m *MockRBACCreator) CreateModuleLoaderServiceAccount(ctx context.Context, mod v1beta1.Module) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "CreateModuleLoaderServiceAccount", ctx, mod) + ret0, _ := ret[0].(error) + return ret0 +} + +// CreateModuleLoaderServiceAccount indicates an expected call of CreateModuleLoaderServiceAccount. +func (mr *MockRBACCreatorMockRecorder) CreateModuleLoaderServiceAccount(ctx, mod interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CreateModuleLoaderServiceAccount", reflect.TypeOf((*MockRBACCreator)(nil).CreateModuleLoaderServiceAccount), ctx, mod) +} diff --git a/internal/rbac/rbac.go b/internal/rbac/rbac.go new file mode 100644 index 000000000..765e3b972 --- /dev/null +++ b/internal/rbac/rbac.go @@ -0,0 +1,84 @@ +package rbac + +import ( + "context" + "fmt" + + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" + "sigs.k8s.io/controller-runtime/pkg/log" + + kmmv1beta1 "github.com/kubernetes-sigs/kernel-module-management/api/v1beta1" +) + +//go:generate mockgen -source=rbac.go -package=rbac -destination=mock_rbac.go + +type RBACCreator interface { + CreateModuleLoaderServiceAccount(ctx context.Context, mod kmmv1beta1.Module) error + CreateDevicePluginServiceAccount(ctx context.Context, mod kmmv1beta1.Module) error +} + +type rbacCreator struct { + client client.Client + scheme *runtime.Scheme +} + +func NewCreator(client client.Client, scheme *runtime.Scheme) RBACCreator { + return &rbacCreator{ + client: client, + scheme: scheme, + } +} + +func (rc *rbacCreator) CreateModuleLoaderServiceAccount(ctx context.Context, mod kmmv1beta1.Module) error { + logger := log.FromContext(ctx) + + sa := &corev1.ServiceAccount{ + ObjectMeta: metav1.ObjectMeta{ + Name: GenerateModuleLoaderServiceAccountName(mod), + Namespace: mod.Namespace, + }, + } + + opRes, err := controllerutil.CreateOrPatch(ctx, rc.client, sa, func() error { + return controllerutil.SetControllerReference(&mod, sa, rc.scheme) + }) + if err != nil { + return fmt.Errorf("cound not create/patch ServiceAccount: %w", err) + } + logger.Info("Created module-loader's ServiceAccount", "name", sa.Name, "result", opRes) + + return nil +} + +func (rc *rbacCreator) CreateDevicePluginServiceAccount(ctx context.Context, mod kmmv1beta1.Module) error { + logger := log.FromContext(ctx) + + sa := &corev1.ServiceAccount{ + ObjectMeta: metav1.ObjectMeta{ + Name: GenerateDevicePluginServiceAccountName(mod), + Namespace: mod.Namespace, + }, + } + + opRes, err := controllerutil.CreateOrPatch(ctx, rc.client, sa, func() error { + return controllerutil.SetControllerReference(&mod, sa, rc.scheme) + }) + if err != nil { + return fmt.Errorf("cound not create/patch ServiceAccount: %w", err) + } + logger.Info("Created device-plugin's ServiceAccount", "name", sa.Name, "result", opRes) + + return nil +} + +func GenerateModuleLoaderServiceAccountName(mod kmmv1beta1.Module) string { + return mod.Name + "-module-loader" +} + +func GenerateDevicePluginServiceAccountName(mod kmmv1beta1.Module) string { + return mod.Name + "-device-plugin" +} diff --git a/internal/rbac/rbac_test.go b/internal/rbac/rbac_test.go new file mode 100644 index 000000000..1bfdf73f5 --- /dev/null +++ b/internal/rbac/rbac_test.go @@ -0,0 +1,218 @@ +package rbac + +import ( + "context" + "errors" + + "github.com/golang/mock/gomock" + corev1 "k8s.io/api/core/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/utils/pointer" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + + kmmv1beta1 "github.com/kubernetes-sigs/kernel-module-management/api/v1beta1" + "github.com/kubernetes-sigs/kernel-module-management/internal/client" +) + +var ( + ctrl *gomock.Controller + clnt *client.MockClient +) + +var _ = Describe("CreateModuleLoaderServiceAccount", func() { + const ( + moduleName = "test-module" + namespace = "namespace" + ) + + var ( + rc RBACCreator + + mod kmmv1beta1.Module + + requestedServiceAccount *corev1.ServiceAccount + + expectedServiceAccount *corev1.ServiceAccount + ) + + ctx := context.Background() + + BeforeEach(func() { + ctrl = gomock.NewController(GinkgoT()) + clnt = client.NewMockClient(ctrl) + rc = NewCreator(clnt, scheme) + + mod = kmmv1beta1.Module{ + TypeMeta: metav1.TypeMeta{ + APIVersion: kmmv1beta1.GroupVersion.String(), + Kind: "Module", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: moduleName, + Namespace: namespace, + }, + } + requestedServiceAccount = &corev1.ServiceAccount{ + ObjectMeta: metav1.ObjectMeta{ + Name: moduleName + "-module-loader", + Namespace: namespace, + }, + } + expectedServiceAccount = &corev1.ServiceAccount{ + ObjectMeta: metav1.ObjectMeta{ + Name: moduleName + "-module-loader", + Namespace: namespace, + OwnerReferences: []metav1.OwnerReference{ + { + APIVersion: mod.APIVersion, + BlockOwnerDeletion: pointer.Bool(true), + Controller: pointer.Bool(true), + Kind: mod.Kind, + Name: moduleName, + UID: mod.UID, + }, + }, + }, + } + }) + + It("should add the default module loader ServiceAccount", func() { + gomock.InOrder( + clnt.EXPECT().Get(ctx, gomock.Any(), requestedServiceAccount).Return(apierrors.NewNotFound(schema.GroupResource{}, "whatever")), + clnt.EXPECT().Create(ctx, expectedServiceAccount).Return(nil), + ) + + err := rc.CreateModuleLoaderServiceAccount(context.Background(), mod) + Expect(err).NotTo(HaveOccurred()) + }) + + It("should return an error when the ServiceAccount fetch fails", func() { + gomock.InOrder( + clnt.EXPECT().Get(ctx, gomock.Any(), requestedServiceAccount).Return(errors.New("some-error")), + ) + + err := rc.CreateModuleLoaderServiceAccount(context.Background(), mod) + Expect(err).To(HaveOccurred()) + }) + + It("should return an error when the ServiceAccount creation fails", func() { + gomock.InOrder( + clnt.EXPECT().Get(ctx, gomock.Any(), requestedServiceAccount).Return(apierrors.NewNotFound(schema.GroupResource{}, "whatever")), + clnt.EXPECT().Create(ctx, expectedServiceAccount).Return(errors.New("some-error")), + ) + + err := rc.CreateModuleLoaderServiceAccount(context.Background(), mod) + Expect(err).To(HaveOccurred()) + }) +}) + +var _ = Describe("CreateDevicePluginServiceAccount", func() { + const ( + moduleName = "test-module" + namespace = "namespace" + ) + + var ( + rc RBACCreator + + mod kmmv1beta1.Module + + requestedServiceAccount *corev1.ServiceAccount + + expectedServiceAccount *corev1.ServiceAccount + ) + + ctx := context.Background() + + BeforeEach(func() { + ctrl = gomock.NewController(GinkgoT()) + clnt = client.NewMockClient(ctrl) + rc = NewCreator(clnt, scheme) + + mod = kmmv1beta1.Module{ + TypeMeta: metav1.TypeMeta{ + APIVersion: kmmv1beta1.GroupVersion.String(), + Kind: "Module", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: moduleName, + Namespace: namespace, + }, + } + requestedServiceAccount = &corev1.ServiceAccount{ + ObjectMeta: metav1.ObjectMeta{ + Name: moduleName + "-device-plugin", + Namespace: namespace, + }, + } + expectedServiceAccount = &corev1.ServiceAccount{ + ObjectMeta: metav1.ObjectMeta{ + Name: moduleName + "-device-plugin", + Namespace: namespace, + OwnerReferences: []metav1.OwnerReference{ + { + APIVersion: mod.APIVersion, + BlockOwnerDeletion: pointer.Bool(true), + Controller: pointer.Bool(true), + Kind: mod.Kind, + Name: moduleName, + UID: mod.UID, + }, + }, + }, + } + }) + + It("should add the default device plugin ServiceAccount", func() { + gomock.InOrder( + clnt.EXPECT().Get(ctx, gomock.Any(), requestedServiceAccount).Return(apierrors.NewNotFound(schema.GroupResource{}, "whatever")), + clnt.EXPECT().Create(ctx, expectedServiceAccount).Return(nil), + ) + + err := rc.CreateDevicePluginServiceAccount(context.Background(), mod) + Expect(err).NotTo(HaveOccurred()) + }) + + It("should return an error when the ServiceAccount fetch fails", func() { + gomock.InOrder( + clnt.EXPECT().Get(ctx, gomock.Any(), requestedServiceAccount).Return(errors.New("some-error")), + ) + + err := rc.CreateDevicePluginServiceAccount(context.Background(), mod) + Expect(err).To(HaveOccurred()) + }) + + It("should return an error when the ServiceAccount creation fails", func() { + gomock.InOrder( + clnt.EXPECT().Get(ctx, gomock.Any(), requestedServiceAccount).Return(apierrors.NewNotFound(schema.GroupResource{}, "whatever")), + clnt.EXPECT().Create(ctx, expectedServiceAccount).Return(errors.New("some-error")), + ) + + err := rc.CreateDevicePluginServiceAccount(context.Background(), mod) + Expect(err).To(HaveOccurred()) + }) +}) + +var _ = Describe("GenerateModuleLoaderServiceAccountName", func() { + mod := kmmv1beta1.Module{ + ObjectMeta: metav1.ObjectMeta{Name: "test-module", Namespace: "namespace"}, + } + + It("should return the default module loader ServiceAccount name", func() { + Expect(GenerateModuleLoaderServiceAccountName(mod)).To(Equal("test-module-module-loader")) + }) +}) + +var _ = Describe("GenerateDevicePluginServiceAccountName", func() { + mod := kmmv1beta1.Module{ + ObjectMeta: metav1.ObjectMeta{Name: "test-module", Namespace: "namespace"}, + } + + It("should return the default device plugin ServiceAccount name", func() { + Expect(GenerateDevicePluginServiceAccountName(mod)).To(Equal("test-module-device-plugin")) + }) +}) diff --git a/internal/rbac/suite_test.go b/internal/rbac/suite_test.go new file mode 100644 index 000000000..049809039 --- /dev/null +++ b/internal/rbac/suite_test.go @@ -0,0 +1,40 @@ +/* +Copyright 2022. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package rbac + +import ( + "testing" + + "github.com/kubernetes-sigs/kernel-module-management/internal/test" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + "k8s.io/apimachinery/pkg/runtime" + //+kubebuilder:scaffold:imports +) + +var scheme *runtime.Scheme + +func TestAPIs(t *testing.T) { + RegisterFailHandler(Fail) + + var err error + + scheme, err = test.TestScheme() + Expect(err).NotTo(HaveOccurred()) + + RunSpecs(t, "RBAC Suite") +} diff --git a/main.go b/main.go index bb6a75480..7dd8dbf56 100644 --- a/main.go +++ b/main.go @@ -30,6 +30,7 @@ import ( "github.com/kubernetes-sigs/kernel-module-management/internal/metrics" "github.com/kubernetes-sigs/kernel-module-management/internal/module" "github.com/kubernetes-sigs/kernel-module-management/internal/preflight" + "github.com/kubernetes-sigs/kernel-module-management/internal/rbac" "github.com/kubernetes-sigs/kernel-module-management/internal/registry" "github.com/kubernetes-sigs/kernel-module-management/internal/statusupdater" "k8s.io/klog/v2/klogr" @@ -124,13 +125,14 @@ func main() { helperAPI := build.NewHelper() makerAPI := job.NewMaker(helperAPI, scheme) buildAPI := job.NewBuildManager(client, makerAPI, helperAPI) + rbacAPI := rbac.NewCreator(client, scheme) daemonAPI := daemonset.NewCreator(client, kernelLabel, scheme) kernelAPI := module.NewKernelMapper() moduleStatusUpdaterAPI := statusupdater.NewModuleStatusUpdater(client, daemonAPI, metricsAPI) preflightStatusUpdaterAPI := statusupdater.NewPreflightStatusUpdater(client) preflightAPI := preflight.NewPreflightAPI(client, buildAPI, registryAPI, preflightStatusUpdaterAPI, kernelAPI) - mc := controllers.NewModuleReconciler(client, buildAPI, daemonAPI, kernelAPI, metricsAPI, filter, registryAPI, moduleStatusUpdaterAPI) + mc := controllers.NewModuleReconciler(client, buildAPI, rbacAPI, daemonAPI, kernelAPI, metricsAPI, filter, registryAPI, moduleStatusUpdaterAPI) if err = mc.SetupWithManager(mgr, kernelLabel); err != nil { setupLogger.Error(err, "unable to create controller", "controller", "Module")