Skip to content

Commit 4dcedf3

Browse files
TomerNewmanTomerNewman
committed
Add DRA status feedback to hub-and-spoke ManifestWork
Add three DRA status feedback JSON path entries (dra.availableNumber, dra.desiredNumber, dra.nodesMatchingSelectorNumber) to the ManifestWork feedback rules, mirroring the existing devicePlugin pattern. Validate DRA hostPath volumes against the same allowlist (/dev, /sys, /var, /opt) used for DevicePlugin volumes, ensuring the hub webhook rejects disallowed mounts before they reach spoke clusters.
1 parent 29c905a commit 4dcedf3

4 files changed

Lines changed: 120 additions & 126 deletions

File tree

‎internal/manifestwork/manifestwork.go‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,18 @@ var moduleStatusJSONPaths = []workv1.JsonPath{
3737
Name: "devicePlugin.nodesMatchingSelectorNumber",
3838
Path: ".status.devicePlugin.nodesMatchingSelectorNumber",
3939
},
40+
{
41+
Name: "dra.availableNumber",
42+
Path: ".status.dra.availableNumber",
43+
},
44+
{
45+
Name: "dra.desiredNumber",
46+
Path: ".status.dra.desiredNumber",
47+
},
48+
{
49+
Name: "dra.nodesMatchingSelectorNumber",
50+
Path: ".status.dra.nodesMatchingSelectorNumber",
51+
},
4052
{
4153
Name: "moduleLoader.availableNumber",
4254
Path: ".status.moduleLoader.availableNumber",

‎internal/manifestwork/manifestwork_test.go‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,22 @@ var (
2626
mockKM *module.MockKernelMapper
2727
)
2828

29+
var _ = Describe("moduleStatusJSONPaths", func() {
30+
It("should contain all expected status feedback paths", func() {
31+
Expect(moduleStatusJSONPaths).To(ConsistOf(
32+
workv1.JsonPath{Name: "devicePlugin.availableNumber", Path: ".status.devicePlugin.availableNumber"},
33+
workv1.JsonPath{Name: "devicePlugin.desiredNumber", Path: ".status.devicePlugin.desiredNumber"},
34+
workv1.JsonPath{Name: "devicePlugin.nodesMatchingSelectorNumber", Path: ".status.devicePlugin.nodesMatchingSelectorNumber"},
35+
workv1.JsonPath{Name: "dra.availableNumber", Path: ".status.dra.availableNumber"},
36+
workv1.JsonPath{Name: "dra.desiredNumber", Path: ".status.dra.desiredNumber"},
37+
workv1.JsonPath{Name: "dra.nodesMatchingSelectorNumber", Path: ".status.dra.nodesMatchingSelectorNumber"},
38+
workv1.JsonPath{Name: "moduleLoader.availableNumber", Path: ".status.moduleLoader.availableNumber"},
39+
workv1.JsonPath{Name: "moduleLoader.desiredNumber", Path: ".status.moduleLoader.desiredNumber"},
40+
workv1.JsonPath{Name: "moduleLoader.nodesMatchingSelectorNumber", Path: ".status.moduleLoader.nodesMatchingSelectorNumber"},
41+
))
42+
})
43+
})
44+
2945
var _ = Describe("GarbageCollect", func() {
3046
BeforeEach(func() {
3147
ctrl = gomock.NewController(GinkgoT())

‎internal/webhook/module.go‎

Lines changed: 14 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -163,8 +163,16 @@ func validateModule(mod *kmmv1beta1.Module, kubeVersion *KubeVersion) (admission
163163
return nil, fmt.Errorf("failed to validate DRA: %v", err)
164164
}
165165

166-
if err := validateDevicePluginVolumes(mod.Spec.DevicePlugin); err != nil {
167-
return nil, fmt.Errorf("failed to validate device plugin volumes: %v", err)
166+
if mod.Spec.DRA != nil {
167+
if err := validateHostPathVolumes("spec.dra", mod.Spec.DRA.Volumes); err != nil {
168+
return nil, fmt.Errorf("failed to validate DRA volumes: %v", err)
169+
}
170+
}
171+
172+
if mod.Spec.DevicePlugin != nil {
173+
if err := validateHostPathVolumes("spec.devicePlugin", mod.Spec.DevicePlugin.Volumes); err != nil {
174+
return nil, fmt.Errorf("failed to validate device plugin volumes: %v", err)
175+
}
168176
}
169177

170178
if mod.Spec.ModuleLoader == nil {
@@ -239,16 +247,12 @@ func isAllowedHostPath(hostPath string) bool {
239247
return false
240248
}
241249

242-
func validateDevicePluginVolumes(dp *kmmv1beta1.DevicePluginSpec) error {
243-
if dp == nil {
244-
return nil
245-
}
246-
247-
for i, vol := range dp.Volumes {
250+
func validateHostPathVolumes(fieldPath string, volumes []corev1.Volume) error {
251+
for i, vol := range volumes {
248252
if vol.HostPath != nil && !isAllowedHostPath(vol.HostPath.Path) {
249253
return fmt.Errorf(
250-
"spec.devicePlugin.volumes[%d]: hostPath %q is not allowed; only /dev, /sys, /var and /opt paths are permitted",
251-
i, vol.HostPath.Path,
254+
"%s.volumes[%d]: hostPath %q is not allowed; only /dev, /sys, /var and /opt paths are permitted",
255+
fieldPath, i, vol.HostPath.Path,
252256
)
253257
}
254258
}

‎internal/webhook/module_test.go‎

Lines changed: 78 additions & 116 deletions
Original file line numberDiff line numberDiff line change
@@ -830,182 +830,144 @@ var _ = Describe("validateDRA", func() {
830830
})
831831
})
832832

833-
var _ = Describe("validateDevicePluginVolumes", func() {
834-
It("should accept nil DevicePlugin", func() {
835-
Expect(validateDevicePluginVolumes(nil)).NotTo(HaveOccurred())
836-
})
837-
838-
It("should accept DevicePlugin with no volumes", func() {
839-
dp := &kmmv1beta1.DevicePluginSpec{
840-
Container: kmmv1beta1.DevicePluginContainerSpec{Image: "img:tag"},
841-
}
842-
Expect(validateDevicePluginVolumes(dp)).NotTo(HaveOccurred())
833+
var _ = Describe("validateHostPathVolumes", func() {
834+
It("should accept empty volume list", func() {
835+
Expect(validateHostPathVolumes("spec.test", nil)).NotTo(HaveOccurred())
836+
Expect(validateHostPathVolumes("spec.test", []v1.Volume{})).NotTo(HaveOccurred())
843837
})
844838

845839
It("should accept non-hostPath volumes", func() {
846-
dp := &kmmv1beta1.DevicePluginSpec{
847-
Container: kmmv1beta1.DevicePluginContainerSpec{Image: "img:tag"},
848-
Volumes: []v1.Volume{
849-
{
850-
Name: "config",
851-
VolumeSource: v1.VolumeSource{
852-
ConfigMap: &v1.ConfigMapVolumeSource{
853-
LocalObjectReference: v1.LocalObjectReference{Name: "my-cm"},
854-
},
855-
},
856-
},
857-
},
858-
}
859-
Expect(validateDevicePluginVolumes(dp)).NotTo(HaveOccurred())
860-
})
861-
862-
It("should accept hostPath volumes under /dev", func() {
863-
dp := &kmmv1beta1.DevicePluginSpec{
864-
Container: kmmv1beta1.DevicePluginContainerSpec{Image: "img:tag"},
865-
Volumes: []v1.Volume{
866-
{
867-
Name: "dev-vfio",
868-
VolumeSource: v1.VolumeSource{
869-
HostPath: &v1.HostPathVolumeSource{Path: "/dev/vfio"},
840+
vols := []v1.Volume{
841+
{
842+
Name: "config",
843+
VolumeSource: v1.VolumeSource{
844+
ConfigMap: &v1.ConfigMapVolumeSource{
845+
LocalObjectReference: v1.LocalObjectReference{Name: "my-cm"},
870846
},
871847
},
872848
},
873849
}
874-
Expect(validateDevicePluginVolumes(dp)).NotTo(HaveOccurred())
850+
Expect(validateHostPathVolumes("spec.test", vols)).NotTo(HaveOccurred())
875851
})
876852

877853
It("should accept hostPath volumes equal to /dev", func() {
878-
dp := &kmmv1beta1.DevicePluginSpec{
879-
Container: kmmv1beta1.DevicePluginContainerSpec{Image: "img:tag"},
880-
Volumes: []v1.Volume{
881-
{
882-
Name: "dev",
883-
VolumeSource: v1.VolumeSource{
884-
HostPath: &v1.HostPathVolumeSource{Path: "/dev"},
885-
},
854+
vols := []v1.Volume{
855+
{
856+
Name: "dev",
857+
VolumeSource: v1.VolumeSource{
858+
HostPath: &v1.HostPathVolumeSource{Path: "/dev"},
886859
},
887860
},
888861
}
889-
Expect(validateDevicePluginVolumes(dp)).NotTo(HaveOccurred())
862+
Expect(validateHostPathVolumes("spec.test", vols)).NotTo(HaveOccurred())
890863
})
891864

892865
It("should accept hostPath volumes under /sys", func() {
893-
dp := &kmmv1beta1.DevicePluginSpec{
894-
Container: kmmv1beta1.DevicePluginContainerSpec{Image: "img:tag"},
895-
Volumes: []v1.Volume{
896-
{
897-
Name: "sys-class",
898-
VolumeSource: v1.VolumeSource{
899-
HostPath: &v1.HostPathVolumeSource{Path: "/sys/class/net"},
900-
},
866+
vols := []v1.Volume{
867+
{
868+
Name: "sys-class",
869+
VolumeSource: v1.VolumeSource{
870+
HostPath: &v1.HostPathVolumeSource{Path: "/sys/class/net"},
901871
},
902872
},
903873
}
904-
Expect(validateDevicePluginVolumes(dp)).NotTo(HaveOccurred())
874+
Expect(validateHostPathVolumes("spec.test", vols)).NotTo(HaveOccurred())
905875
})
906876

907877
It("should accept hostPath volumes under /var", func() {
908-
dp := &kmmv1beta1.DevicePluginSpec{
909-
Container: kmmv1beta1.DevicePluginContainerSpec{Image: "img:tag"},
910-
Volumes: []v1.Volume{
911-
{
912-
Name: "var-lib",
913-
VolumeSource: v1.VolumeSource{
914-
HostPath: &v1.HostPathVolumeSource{Path: "/var/lib/kubelet/device-plugins"},
915-
},
878+
vols := []v1.Volume{
879+
{
880+
Name: "var-lib",
881+
VolumeSource: v1.VolumeSource{
882+
HostPath: &v1.HostPathVolumeSource{Path: "/var/lib/kubelet/device-plugins"},
916883
},
917884
},
918885
}
919-
Expect(validateDevicePluginVolumes(dp)).NotTo(HaveOccurred())
886+
Expect(validateHostPathVolumes("spec.test", vols)).NotTo(HaveOccurred())
920887
})
921888

922889
It("should accept hostPath volumes under /opt", func() {
923-
dp := &kmmv1beta1.DevicePluginSpec{
924-
Container: kmmv1beta1.DevicePluginContainerSpec{Image: "img:tag"},
925-
Volumes: []v1.Volume{
926-
{
927-
Name: "opt-lib",
928-
VolumeSource: v1.VolumeSource{
929-
HostPath: &v1.HostPathVolumeSource{Path: "/opt/lib/firmware"},
930-
},
890+
vols := []v1.Volume{
891+
{
892+
Name: "opt-lib",
893+
VolumeSource: v1.VolumeSource{
894+
HostPath: &v1.HostPathVolumeSource{Path: "/opt/lib/firmware"},
931895
},
932896
},
933897
}
934-
Expect(validateDevicePluginVolumes(dp)).NotTo(HaveOccurred())
898+
Expect(validateHostPathVolumes("spec.test", vols)).NotTo(HaveOccurred())
935899
})
936900

937901
It("should reject hostPath volumes outside allowed paths", func() {
938-
dp := &kmmv1beta1.DevicePluginSpec{
939-
Container: kmmv1beta1.DevicePluginContainerSpec{Image: "img:tag"},
940-
Volumes: []v1.Volume{
941-
{
942-
Name: "root",
943-
VolumeSource: v1.VolumeSource{
944-
HostPath: &v1.HostPathVolumeSource{Path: "/"},
945-
},
902+
vols := []v1.Volume{
903+
{
904+
Name: "root",
905+
VolumeSource: v1.VolumeSource{
906+
HostPath: &v1.HostPathVolumeSource{Path: "/"},
946907
},
947908
},
948909
}
949-
Expect(validateDevicePluginVolumes(dp)).To(MatchError(ContainSubstring("not allowed")))
910+
Expect(validateHostPathVolumes("spec.test", vols)).To(MatchError(ContainSubstring("not allowed")))
950911
})
951912

952913
It("should reject hostPath volumes under /etc", func() {
953-
dp := &kmmv1beta1.DevicePluginSpec{
954-
Container: kmmv1beta1.DevicePluginContainerSpec{Image: "img:tag"},
955-
Volumes: []v1.Volume{
956-
{
957-
Name: "etc",
958-
VolumeSource: v1.VolumeSource{
959-
HostPath: &v1.HostPathVolumeSource{Path: "/etc/config"},
960-
},
914+
vols := []v1.Volume{
915+
{
916+
Name: "etc",
917+
VolumeSource: v1.VolumeSource{
918+
HostPath: &v1.HostPathVolumeSource{Path: "/etc/config"},
961919
},
962920
},
963921
}
964-
Expect(validateDevicePluginVolumes(dp)).To(MatchError(ContainSubstring("not allowed")))
922+
Expect(validateHostPathVolumes("spec.test", vols)).To(MatchError(ContainSubstring("not allowed")))
965923
})
966924

967925
It("should reject hostPath with prefix trick like /devious", func() {
968-
dp := &kmmv1beta1.DevicePluginSpec{
969-
Container: kmmv1beta1.DevicePluginContainerSpec{Image: "img:tag"},
970-
Volumes: []v1.Volume{
971-
{
972-
Name: "tricky",
973-
VolumeSource: v1.VolumeSource{
974-
HostPath: &v1.HostPathVolumeSource{Path: "/devious"},
975-
},
926+
vols := []v1.Volume{
927+
{
928+
Name: "tricky",
929+
VolumeSource: v1.VolumeSource{
930+
HostPath: &v1.HostPathVolumeSource{Path: "/devious"},
976931
},
977932
},
978933
}
979-
Expect(validateDevicePluginVolumes(dp)).To(MatchError(ContainSubstring("not allowed")))
934+
Expect(validateHostPathVolumes("spec.test", vols)).To(MatchError(ContainSubstring("not allowed")))
980935
})
981936

982937
It("should reject hostPath with path traversal like /dev/../etc", func() {
983-
dp := &kmmv1beta1.DevicePluginSpec{
984-
Container: kmmv1beta1.DevicePluginContainerSpec{Image: "img:tag"},
985-
Volumes: []v1.Volume{
986-
{
987-
Name: "traversal",
988-
VolumeSource: v1.VolumeSource{
989-
HostPath: &v1.HostPathVolumeSource{Path: "/dev/../etc/shadow"},
990-
},
938+
vols := []v1.Volume{
939+
{
940+
Name: "traversal",
941+
VolumeSource: v1.VolumeSource{
942+
HostPath: &v1.HostPathVolumeSource{Path: "/dev/../etc/shadow"},
991943
},
992944
},
993945
}
994-
Expect(validateDevicePluginVolumes(dp)).To(MatchError(ContainSubstring("not allowed")))
946+
Expect(validateHostPathVolumes("spec.test", vols)).To(MatchError(ContainSubstring("not allowed")))
995947
})
996948

997949
It("should accept hostPath with redundant slashes under /sys", func() {
998-
dp := &kmmv1beta1.DevicePluginSpec{
999-
Container: kmmv1beta1.DevicePluginContainerSpec{Image: "img:tag"},
1000-
Volumes: []v1.Volume{
1001-
{
1002-
Name: "sys-clean",
1003-
VolumeSource: v1.VolumeSource{
1004-
HostPath: &v1.HostPathVolumeSource{Path: "/sys//class//net"},
1005-
},
950+
vols := []v1.Volume{
951+
{
952+
Name: "sys-clean",
953+
VolumeSource: v1.VolumeSource{
954+
HostPath: &v1.HostPathVolumeSource{Path: "/sys//class//net"},
955+
},
956+
},
957+
}
958+
Expect(validateHostPathVolumes("spec.test", vols)).NotTo(HaveOccurred())
959+
})
960+
961+
It("should include fieldPath in error message", func() {
962+
vols := []v1.Volume{
963+
{
964+
Name: "bad",
965+
VolumeSource: v1.VolumeSource{
966+
HostPath: &v1.HostPathVolumeSource{Path: "/etc/secret"},
1006967
},
1007968
},
1008969
}
1009-
Expect(validateDevicePluginVolumes(dp)).NotTo(HaveOccurred())
970+
Expect(validateHostPathVolumes("spec.dra", vols)).To(MatchError(ContainSubstring("spec.dra.volumes[0]")))
971+
Expect(validateHostPathVolumes("spec.devicePlugin", vols)).To(MatchError(ContainSubstring("spec.devicePlugin.volumes[0]")))
1010972
})
1011973
})

0 commit comments

Comments
 (0)