Skip to content

Commit d1126b6

Browse files
k8s-ci-robotk8s-ci-robot
authored
Merge pull request kubernetes#130037 from jsafrane/selinux-controller-translator
selinux: add a new SELinux translator to the controller
2 parents 2b3da7d + 2050d6f commit d1126b6

File tree

6 files changed

+407
-65
lines changed

6 files changed

+407
-65
lines changed

‎pkg/controller/volume/selinuxwarning/cache/volumecache.go‎

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ import (
2323
v1 "k8s.io/api/core/v1"
2424
"k8s.io/client-go/tools/cache"
2525
"k8s.io/klog/v2"
26+
"k8s.io/kubernetes/pkg/controller/volume/selinuxwarning/translator"
2627
)
2728

2829
const (
@@ -51,17 +52,19 @@ type VolumeCache interface {
5152
// VolumeCache stores all volumes used by Pods and their properties that the controller needs to track,
5253
// like SELinux labels and SELinuxChangePolicies.
5354
type volumeCache struct {
54-
mutex sync.RWMutex
55+
mutex sync.RWMutex
56+
seLinuxTranslator *translator.ControllerSELinuxTranslator
5557
// All volumes of all existing Pods.
5658
volumes map[v1.UniqueVolumeName]usedVolume
5759
}
5860

5961
var _ VolumeCache = &volumeCache{}
6062

6163
// NewVolumeLabelCache creates a new VolumeCache.
62-
func NewVolumeLabelCache() VolumeCache {
64+
func NewVolumeLabelCache(seLinuxTranslator *translator.ControllerSELinuxTranslator) VolumeCache {
6365
return &volumeCache{
64-
volumes: make(map[v1.UniqueVolumeName]usedVolume),
66+
seLinuxTranslator: seLinuxTranslator,
67+
volumes: make(map[v1.UniqueVolumeName]usedVolume),
6568
}
6669
}
6770

@@ -137,7 +140,7 @@ func (c *volumeCache) AddVolume(logger klog.Logger, volumeName v1.UniqueVolumeNa
137140
OtherPropertyValue: string(changePolicy),
138141
})
139142
}
140-
if otherPodInfo.seLinuxLabel != label {
143+
if c.seLinuxTranslator.Conflicts(otherPodInfo.seLinuxLabel, label) {
141144
// Send conflict to both pods
142145
conflicts = append(conflicts, Conflict{
143146
PropertyName: "SELinuxLabel",
@@ -248,7 +251,7 @@ func (c *volumeCache) SendConflicts(logger klog.Logger, ch chan<- Conflict) {
248251
OtherPropertyValue: string(otherPodInfo.changePolicy),
249252
}
250253
}
251-
if podInfo.seLinuxLabel != otherPodInfo.seLinuxLabel {
254+
if c.seLinuxTranslator.Conflicts(podInfo.seLinuxLabel, otherPodInfo.seLinuxLabel) {
252255
ch <- Conflict{
253256
PropertyName: "SELinuxLabel",
254257
EventReason: "SELinuxLabelConflict",

‎pkg/controller/volume/selinuxwarning/cache/volumecache_test.go‎

Lines changed: 125 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ import (
2525
"k8s.io/client-go/tools/cache"
2626
"k8s.io/klog/v2"
2727
"k8s.io/klog/v2/ktesting"
28+
"k8s.io/kubernetes/pkg/controller/volume/selinuxwarning/translator"
2829
)
2930

3031
func getTestLoggers(t *testing.T) (klog.Logger, klog.Logger) {
@@ -47,7 +48,8 @@ func sortConflicts(conflicts []Conflict) {
4748
// Delete all items in a bigger cache and check it's empty
4849
func TestVolumeCache_DeleteAll(t *testing.T) {
4950
var podsToDelete []cache.ObjectName
50-
c := NewVolumeLabelCache().(*volumeCache)
51+
seLinuxTranslator := &translator.ControllerSELinuxTranslator{}
52+
c := NewVolumeLabelCache(seLinuxTranslator).(*volumeCache)
5153
logger, dumpLogger := getTestLoggers(t)
5254

5355
// Arrange: add a lot of volumes to the cache
@@ -110,42 +112,70 @@ func TestVolumeCache_AddVolumeSendConflicts(t *testing.T) {
110112
podNamespace: "ns1",
111113
podName: "pod1-mountOption",
112114
volumeName: "vol1",
113-
label: "label1",
115+
label: "system_u:system_r:label1",
114116
changePolicy: v1.SELinuxChangePolicyMountOption,
115117
},
116118
{
117119
podNamespace: "ns2",
118120
podName: "pod2-recursive",
119121
volumeName: "vol2",
120-
label: "label2",
122+
label: "system_u:system_r:label2",
121123
changePolicy: v1.SELinuxChangePolicyRecursive,
122124
},
123125
{
124126
podNamespace: "ns3",
125127
podName: "pod3-1",
126128
volumeName: "vol3", // vol3 is used by 2 pods with the same label + recursive policy
127-
label: "label3",
129+
label: "system_u:system_r:label3",
128130
changePolicy: v1.SELinuxChangePolicyRecursive,
129131
},
130132
{
131133
podNamespace: "ns3",
132134
podName: "pod3-2",
133135
volumeName: "vol3", // vol3 is used by 2 pods with the same label + recursive policy
134-
label: "label3",
136+
label: "system_u:system_r:label3",
135137
changePolicy: v1.SELinuxChangePolicyRecursive,
136138
},
137139
{
138140
podNamespace: "ns4",
139141
podName: "pod4-1",
140142
volumeName: "vol4", // vol4 is used by 2 pods with the same label + mount policy
141-
label: "label4",
143+
label: "system_u:system_r:label4",
142144
changePolicy: v1.SELinuxChangePolicyMountOption,
143145
},
144146
{
145147
podNamespace: "ns4",
146148
podName: "pod4-2",
147149
volumeName: "vol4", // vol4 is used by 2 pods with the same label + mount policy
148-
label: "label4",
150+
label: "system_u:system_r:label4",
151+
changePolicy: v1.SELinuxChangePolicyMountOption,
152+
},
153+
{
154+
podNamespace: "ns5",
155+
podName: "pod5",
156+
volumeName: "vol5", // vol5 has no user and role
157+
label: "::label5",
158+
changePolicy: v1.SELinuxChangePolicyMountOption,
159+
},
160+
{
161+
podNamespace: "ns6",
162+
podName: "pod6",
163+
volumeName: "vol6", // vol6 has no user
164+
label: ":system_r:label6",
165+
changePolicy: v1.SELinuxChangePolicyMountOption,
166+
},
167+
{
168+
podNamespace: "ns7",
169+
podName: "pod7",
170+
volumeName: "vol7", // vol7 has no user and role, but has categories
171+
label: "::label7:c0,c1",
172+
changePolicy: v1.SELinuxChangePolicyMountOption,
173+
},
174+
{
175+
podNamespace: "ns8",
176+
podName: "pod8",
177+
volumeName: "vol8", // vol has no label
178+
label: "",
149179
changePolicy: v1.SELinuxChangePolicyMountOption,
150180
},
151181
}
@@ -163,7 +193,7 @@ func TestVolumeCache_AddVolumeSendConflicts(t *testing.T) {
163193
podNamespace: "testns",
164194
podName: "testpod",
165195
volumeName: "vol-new",
166-
label: "label-new",
196+
label: "system_u:system_r:label-new",
167197
changePolicy: v1.SELinuxChangePolicyMountOption,
168198
},
169199
expectedConflicts: nil,
@@ -175,7 +205,7 @@ func TestVolumeCache_AddVolumeSendConflicts(t *testing.T) {
175205
podNamespace: "testns",
176206
podName: "testpod",
177207
volumeName: "vol-new",
178-
label: "label-new",
208+
label: "system_u:system_r:label-new",
179209
changePolicy: v1.SELinuxChangePolicyMountOption,
180210
},
181211
expectedConflicts: nil,
@@ -187,7 +217,7 @@ func TestVolumeCache_AddVolumeSendConflicts(t *testing.T) {
187217
podNamespace: "testns",
188218
podName: "testpod",
189219
volumeName: "vol1",
190-
label: "label1",
220+
label: "system_u:system_r:label1",
191221
changePolicy: v1.SELinuxChangePolicyMountOption,
192222
},
193223
expectedConflicts: nil,
@@ -199,17 +229,17 @@ func TestVolumeCache_AddVolumeSendConflicts(t *testing.T) {
199229
podNamespace: "testns",
200230
podName: "testpod",
201231
volumeName: "vol1",
202-
label: "label-new",
232+
label: "system_u:system_r:label-new",
203233
changePolicy: v1.SELinuxChangePolicyMountOption,
204234
},
205235
expectedConflicts: []Conflict{
206236
{
207237
PropertyName: "SELinuxLabel",
208238
EventReason: "SELinuxLabelConflict",
209239
Pod: cache.ObjectName{Namespace: "testns", Name: "testpod"},
210-
PropertyValue: "label-new",
240+
PropertyValue: "system_u:system_r:label-new",
211241
OtherPod: cache.ObjectName{Namespace: "ns1", Name: "pod1-mountOption"},
212-
OtherPropertyValue: "label1",
242+
OtherPropertyValue: "system_u:system_r:label1",
213243
},
214244
},
215245
},
@@ -220,7 +250,7 @@ func TestVolumeCache_AddVolumeSendConflicts(t *testing.T) {
220250
podNamespace: "testns",
221251
podName: "testpod",
222252
volumeName: "vol1",
223-
label: "label1",
253+
label: "system_u:system_r:label1",
224254
changePolicy: v1.SELinuxChangePolicyRecursive,
225255
},
226256
expectedConflicts: []Conflict{
@@ -241,7 +271,7 @@ func TestVolumeCache_AddVolumeSendConflicts(t *testing.T) {
241271
podNamespace: "testns",
242272
podName: "testpod",
243273
volumeName: "vol1",
244-
label: "label-new",
274+
label: "system_u:system_r:label-new",
245275
changePolicy: v1.SELinuxChangePolicyRecursive,
246276
},
247277
expectedConflicts: []Conflict{
@@ -257,9 +287,9 @@ func TestVolumeCache_AddVolumeSendConflicts(t *testing.T) {
257287
PropertyName: "SELinuxLabel",
258288
EventReason: "SELinuxLabelConflict",
259289
Pod: cache.ObjectName{Namespace: "testns", Name: "testpod"},
260-
PropertyValue: "label-new",
290+
PropertyValue: "system_u:system_r:label-new",
261291
OtherPod: cache.ObjectName{Namespace: "ns1", Name: "pod1-mountOption"},
262-
OtherPropertyValue: "label1",
292+
OtherPropertyValue: "system_u:system_r:label1",
263293
},
264294
},
265295
},
@@ -271,7 +301,7 @@ func TestVolumeCache_AddVolumeSendConflicts(t *testing.T) {
271301
podNamespace: "ns2",
272302
podName: "pod2-recursive",
273303
volumeName: "vol2", // there is no other pod that uses vol2 -> change of policy and label is possible
274-
label: "label-new", // was label2 in the original pod2
304+
label: "system_u:system_r:label-new", // was label2 in the original pod2
275305
changePolicy: v1.SELinuxChangePolicyMountOption, // was Recursive in the original pod2
276306
},
277307
expectedConflicts: nil,
@@ -284,7 +314,7 @@ func TestVolumeCache_AddVolumeSendConflicts(t *testing.T) {
284314
podNamespace: "ns3",
285315
podName: "pod3-1",
286316
volumeName: "vol3", // vol3 is used by pod3-2 with label3 and Recursive policy
287-
label: "label-new", // Technically, it's not possible to change a label of an existing pod, but we still check for conflicts
317+
label: "system_u:system_r:label-new", // Technically, it's not possible to change a label of an existing pod, but we still check for conflicts
288318
changePolicy: v1.SELinuxChangePolicyMountOption, // ChangePolicy change can happen when CSIDriver is updated from SELinuxMount: false to SELinuxMount: true
289319
},
290320
expectedConflicts: []Conflict{
@@ -300,18 +330,88 @@ func TestVolumeCache_AddVolumeSendConflicts(t *testing.T) {
300330
PropertyName: "SELinuxLabel",
301331
EventReason: "SELinuxLabelConflict",
302332
Pod: cache.ObjectName{Namespace: "ns3", Name: "pod3-1"},
303-
PropertyValue: "label-new",
333+
PropertyValue: "system_u:system_r:label-new",
304334
OtherPod: cache.ObjectName{Namespace: "ns3", Name: "pod3-2"},
305-
OtherPropertyValue: "label3",
335+
OtherPropertyValue: "system_u:system_r:label3",
306336
},
307337
},
308338
},
339+
{
340+
name: "existing volume in a new pod with existing policy and new incomparable label (missing user and role)",
341+
initialPods: existingPods,
342+
podToAdd: podWithVolume{
343+
podNamespace: "testns",
344+
podName: "testpod",
345+
volumeName: "vol5",
346+
label: "system_u:system_r:label5",
347+
changePolicy: v1.SELinuxChangePolicyMountOption,
348+
},
349+
expectedConflicts: []Conflict{},
350+
},
351+
{
352+
name: "existing volume in a new pod with conflicting policy with incomparable parts",
353+
initialPods: existingPods,
354+
podToAdd: podWithVolume{
355+
podNamespace: "testns",
356+
podName: "testpod",
357+
volumeName: "vol5",
358+
label: "::label6",
359+
changePolicy: v1.SELinuxChangePolicyMountOption,
360+
},
361+
expectedConflicts: []Conflict{
362+
{
363+
PropertyName: "SELinuxLabel",
364+
EventReason: "SELinuxLabelConflict",
365+
Pod: cache.ObjectName{Namespace: "testns", Name: "testpod"},
366+
PropertyValue: "::label6",
367+
OtherPod: cache.ObjectName{Namespace: "ns5", Name: "pod5"},
368+
OtherPropertyValue: "::label5",
369+
},
370+
},
371+
},
372+
{
373+
name: "existing volume in a new pod with existing policy and new incomparable label (missing user)",
374+
initialPods: existingPods,
375+
podToAdd: podWithVolume{
376+
podNamespace: "testns",
377+
podName: "testpod",
378+
volumeName: "vol6",
379+
label: "system_u::label6",
380+
changePolicy: v1.SELinuxChangePolicyMountOption,
381+
},
382+
expectedConflicts: []Conflict{},
383+
},
384+
{
385+
name: "existing volume in a new pod with existing policy and new incomparable label (missing categories)",
386+
initialPods: existingPods,
387+
podToAdd: podWithVolume{
388+
podNamespace: "testns",
389+
podName: "testpod",
390+
volumeName: "vol7",
391+
label: "system_u:system_r:label7",
392+
changePolicy: v1.SELinuxChangePolicyMountOption,
393+
},
394+
expectedConflicts: []Conflict{},
395+
},
396+
{
397+
name: "existing volume in a new pod with existing policy and new incomparable label (missing everything)",
398+
initialPods: existingPods,
399+
podToAdd: podWithVolume{
400+
podNamespace: "testns",
401+
podName: "testpod",
402+
volumeName: "vol8",
403+
label: "system_u:system_r:label8",
404+
changePolicy: v1.SELinuxChangePolicyMountOption,
405+
},
406+
expectedConflicts: []Conflict{},
407+
},
309408
}
310409
for _, tt := range tests {
311410
t.Run(tt.name, func(t *testing.T) {
312411
logger, dumpLogger := getTestLoggers(t)
313412
// Arrange: add initial pods to the cache
314-
c := NewVolumeLabelCache().(*volumeCache)
413+
seLinuxTranslator := &translator.ControllerSELinuxTranslator{}
414+
c := NewVolumeLabelCache(seLinuxTranslator).(*volumeCache)
315415
for _, podToAdd := range tt.initialPods {
316416
conflicts := c.AddVolume(logger, podToAdd.volumeName, cache.ObjectName{Namespace: podToAdd.podNamespace, Name: podToAdd.podName}, podToAdd.label, podToAdd.changePolicy, "csiDriver1")
317417
if len(conflicts) != 0 {
@@ -328,6 +428,7 @@ func TestVolumeCache_AddVolumeSendConflicts(t *testing.T) {
328428
sortConflicts(expectedConflicts)
329429
if !reflect.DeepEqual(conflicts, expectedConflicts) {
330430
t.Errorf("AddVolume returned unexpected conflicts: %+v", conflicts)
431+
t.Logf("Expected conflicts: %+v", expectedConflicts)
331432
c.dump(dumpLogger)
332433
}
333434
// Expect the pod + volume to be present in the cache
@@ -370,7 +471,8 @@ func TestVolumeCache_AddVolumeSendConflicts(t *testing.T) {
370471
}
371472

372473
func TestVolumeCache_GetPodsForCSIDriver(t *testing.T) {
373-
c := NewVolumeLabelCache().(*volumeCache)
474+
seLinuxTranslator := &translator.ControllerSELinuxTranslator{}
475+
c := NewVolumeLabelCache(seLinuxTranslator).(*volumeCache)
374476
logger, dumpLogger := getTestLoggers(t)
375477

376478
existingPods := map[string][]podWithVolume{

‎pkg/controller/volume/selinuxwarning/selinux_warning_controller.go‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@ import (
4444
"k8s.io/kubernetes/pkg/controller/volume/attachdetach/util"
4545
"k8s.io/kubernetes/pkg/controller/volume/common"
4646
volumecache "k8s.io/kubernetes/pkg/controller/volume/selinuxwarning/cache"
47+
"k8s.io/kubernetes/pkg/controller/volume/selinuxwarning/translator"
4748
"k8s.io/kubernetes/pkg/volume"
4849
"k8s.io/kubernetes/pkg/volume/csi"
4950
"k8s.io/kubernetes/pkg/volume/csimigration"
@@ -74,7 +75,7 @@ type Controller struct {
7475
vpm *volume.VolumePluginMgr
7576
cmpm csimigration.PluginManager
7677
csiTranslator csimigration.InTreeToCSITranslator
77-
seLinuxTranslator volumeutil.SELinuxLabelTranslator
78+
seLinuxTranslator *translator.ControllerSELinuxTranslator
7879
eventBroadcaster record.EventBroadcaster
7980
eventRecorder record.EventRecorder
8081
queue workqueue.TypedRateLimitingInterface[cache.ObjectName]
@@ -95,6 +96,8 @@ func NewController(
9596

9697
eventBroadcaster := record.NewBroadcaster(record.WithContext(ctx))
9798
recorder := eventBroadcaster.NewRecorder(scheme.Scheme, v1.EventSource{Component: "selinux_warning"})
99+
seLinuxTranslator := &translator.ControllerSELinuxTranslator{}
100+
98101
c := &Controller{
99102
kubeClient: kubeClient,
100103
podLister: podInformer.Lister(),
@@ -107,7 +110,7 @@ func NewController(
107110
csiDriverLister: csiDriverInformer.Lister(),
108111
csiDriversSynced: csiDriverInformer.Informer().HasSynced,
109112
vpm: &volume.VolumePluginMgr{},
110-
seLinuxTranslator: volumeutil.NewSELinuxLabelTranslator(),
113+
seLinuxTranslator: seLinuxTranslator,
111114

112115
eventBroadcaster: eventBroadcaster,
113116
eventRecorder: recorder,
@@ -117,7 +120,7 @@ func NewController(
117120
Name: "selinux_warning",
118121
},
119122
),
120-
labelCache: volumecache.NewVolumeLabelCache(),
123+
labelCache: volumecache.NewVolumeLabelCache(seLinuxTranslator),
121124
}
122125

123126
err := c.vpm.InitPlugins(plugins, prober, c)

0 commit comments

Comments
 (0)