Skip to content

Commit 4e3ef66

Browse files
mytreya-rhmytreya-rh
committed
Add CSI VOLUME_MOUNT_GROUP support for FSGroup-based file ownership
Implements the CSI NodeServiceCapability RPC_VOLUME_MOUNT_GROUP so that mounted secret files are chown'd to the pod's FSGroup. This allows secrets to be not world-readable in non-root containers. 1. nodeServer::NodeGetCapabilities() advertise VOLUME_MOUNT_GROUP 2. nodeServer::NodePublishVolume() get POD's FSGroup if any from: req.VolumeCapability.GetMount().GetVolumeMountGroup() 3. pass the fsgroup onto (writer.go) WritePayloads() 4. include the FSGroup in the FileProjection struct (rename FileProjection::FSUser as FSGroup) 5. change AtomicWriter::writePayloadToDir() to chown the group based on FSGroup 6. Add relevant Unit tests, and e2eprovider tests 7. Bit of refactoring in the unit and e2eprovider tests to make them more terse
1 parent 9e0b295 commit 4e3ef66

15 files changed

Lines changed: 418 additions & 281 deletions

‎pkg/secrets-store/nodeserver.go‎

Lines changed: 19 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ import (
2727
"time"
2828

2929
internalerrors "sigs.k8s.io/secrets-store-csi-driver/pkg/errors"
30+
"sigs.k8s.io/secrets-store-csi-driver/pkg/util/fileutil"
3031

3132
"github.com/container-storage-interface/spec/lib/go/csi"
3233
"google.golang.org/grpc/codes"
@@ -160,7 +161,14 @@ func (ns *nodeServer) NodePublishVolume(ctx context.Context, req *csi.NodePublis
160161
return &csi.NodePublishVolumeResponse{}, nil
161162
}
162163

163-
klog.V(2).InfoS("node publish volume", "target", targetPath, "volumeId", volumeID, "mount flags", mountFlags)
164+
// Group ID to Chown the volume contents to
165+
mountVol := req.GetVolumeCapability().GetMount()
166+
klog.V(2).InfoS("node publish volume", "target", targetPath, "volumeId", volumeID, "mount flags", mountFlags, "volumeMountGroup", mountVol.GetVolumeMountGroup())
167+
gid, err := fileutil.ParseFSGroup(mountVol.GetVolumeMountGroup())
168+
if err != nil {
169+
klog.ErrorS(err, "failed to mount secrets store object content due to invalid FSGroup", "pod", klog.ObjectRef{Namespace: podNamespace, Name: podName}, "fsGroup", mountVol.GetVolumeMountGroup())
170+
return nil, status.Errorf(codes.InvalidArgument, "error parsing FSGroup: %v", err)
171+
}
164172

165173
if isMockProvider(providerName) {
166174
// mock provider is used only for running sanity tests against the driver
@@ -241,7 +249,7 @@ func (ns *nodeServer) NodePublishVolume(ctx context.Context, req *csi.NodePublis
241249
}
242250
mounted = true
243251
var objectVersions map[string]string
244-
if objectVersions, errorReason, err = ns.mountSecretsStoreObjectContent(ctx, providerName, string(parametersStr), string(secretStr), targetPath, string(permissionStr), podName); err != nil {
252+
if objectVersions, errorReason, err = ns.mountSecretsStoreObjectContent(ctx, providerName, string(parametersStr), string(secretStr), targetPath, string(permissionStr), podName, gid); err != nil {
245253
klog.ErrorS(err, "failed to mount secrets store object content", "pod", klog.ObjectRef{Namespace: podNamespace, Name: podName}, "isRemountRequest", isRemountRequest)
246254
if isRemountRequest {
247255
// Mask error until fix available for https://github.com/kubernetes/kubernetes/issues/121271
@@ -342,7 +350,7 @@ func (ns *nodeServer) NodeUnstageVolume(ctx context.Context, req *csi.NodeUnstag
342350
return &csi.NodeUnstageVolumeResponse{}, nil
343351
}
344352

345-
func (ns *nodeServer) mountSecretsStoreObjectContent(ctx context.Context, providerName, attributes, secrets, targetPath, permission, podName string) (map[string]string, string, error) {
353+
func (ns *nodeServer) mountSecretsStoreObjectContent(ctx context.Context, providerName, attributes, secrets, targetPath, permission, podName string, gid int) (map[string]string, string, error) {
346354
if len(attributes) == 0 {
347355
return nil, "", errors.New("missing attributes")
348356
}
@@ -360,7 +368,7 @@ func (ns *nodeServer) mountSecretsStoreObjectContent(ctx context.Context, provid
360368

361369
klog.InfoS("Using gRPC client", "provider", providerName, "pod", podName)
362370

363-
return MountContent(ctx, client, attributes, secrets, targetPath, permission, nil)
371+
return MountContent(ctx, client, attributes, secrets, targetPath, permission, nil, gid)
364372
}
365373

366374
func (ns *nodeServer) NodeGetInfo(ctx context.Context, req *csi.NodeGetInfoRequest) (*csi.NodeGetInfoResponse, error) {
@@ -384,6 +392,13 @@ func (ns *nodeServer) NodeGetCapabilities(ctx context.Context, req *csi.NodeGetC
384392
},
385393
},
386394
},
395+
{
396+
Type: &csi.NodeServiceCapability_Rpc{
397+
Rpc: &csi.NodeServiceCapability_RPC{
398+
Type: csi.NodeServiceCapability_RPC_VOLUME_MOUNT_GROUP,
399+
},
400+
},
401+
},
387402
}
388403

389404
return &csi.NodeGetCapabilitiesResponse{

‎pkg/secrets-store/nodeserver_test.go‎

Lines changed: 106 additions & 173 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,45 @@ func testNodeServer(t *testing.T, client client.Client, reporter StatsReporter,
5757
return newNodeServer("testnode", mount.NewFakeMounter([]mount.MountPoint{}), providerClients, client, client, reporter, rotationConfig)
5858
}
5959

60+
func getInitObjects(customize func(*secretsstorev1.SecretProviderClass)) []client.Object {
61+
var spc = &secretsstorev1.SecretProviderClass{
62+
ObjectMeta: metav1.ObjectMeta{
63+
Name: "provider1",
64+
Namespace: "default",
65+
},
66+
Spec: secretsstorev1.SecretProviderClassSpec{
67+
Provider: "provider1",
68+
Parameters: map[string]string{"parameter1": "value1"},
69+
},
70+
}
71+
customize(spc)
72+
var initObjects = []client.Object{
73+
spc,
74+
}
75+
return initObjects
76+
}
77+
78+
func getRequest(t *testing.T, customize func(*csi.NodePublishVolumeRequest)) *csi.NodePublishVolumeRequest {
79+
var request = &csi.NodePublishVolumeRequest{
80+
VolumeCapability: &csi.VolumeCapability{
81+
AccessType: &csi.VolumeCapability_Mount{
82+
Mount: &csi.VolumeCapability_MountVolume{},
83+
},
84+
},
85+
VolumeId: "testvolid1",
86+
VolumeContext: map[string]string{
87+
"secretProviderClass": "provider1",
88+
csiPodName: "pod1",
89+
csiPodNamespace: "default",
90+
csiPodUID: "poduid1",
91+
},
92+
TargetPath: targetPath(t),
93+
Readonly: true,
94+
}
95+
customize(request)
96+
return request
97+
}
98+
6099
func TestNodePublishVolume_Errors(t *testing.T) {
61100
tests := []struct {
62101
name string
@@ -104,117 +143,59 @@ func TestNodePublishVolume_Errors(t *testing.T) {
104143
want: codes.Unknown,
105144
},
106145
{
107-
name: "spc missing",
108-
nodePublishVolReq: &csi.NodePublishVolumeRequest{
109-
VolumeCapability: &csi.VolumeCapability{},
110-
VolumeId: "testvolid1",
111-
TargetPath: targetPath(t),
112-
VolumeContext: map[string]string{"secretProviderClass": "provider1", csiPodName: "pod1", csiPodNamespace: "default", csiPodUID: "poduid1", "providerName": "provider1"},
113-
Readonly: true,
114-
},
115-
initObjects: []client.Object{
116-
&secretsstorev1.SecretProviderClass{
117-
ObjectMeta: metav1.ObjectMeta{
118-
Name: "provider1",
119-
Namespace: "incorrect_namespace",
120-
},
121-
Spec: secretsstorev1.SecretProviderClassSpec{
122-
Provider: "provider1",
123-
Parameters: map[string]string{"parameter1": "value1"},
124-
},
125-
},
126-
},
146+
name: "spc missing",
147+
nodePublishVolReq: getRequest(t, func(*csi.NodePublishVolumeRequest) {}),
148+
initObjects: getInitObjects(func(s *secretsstorev1.SecretProviderClass) {
149+
s.ObjectMeta.Namespace = "incorrect_namespace"
150+
}),
127151
want: codes.Unknown,
128152
},
129153
{
130-
name: "provider not set in secret provider class",
131-
nodePublishVolReq: &csi.NodePublishVolumeRequest{
132-
VolumeCapability: &csi.VolumeCapability{},
133-
VolumeId: "testvolid1",
134-
TargetPath: targetPath(t),
135-
VolumeContext: map[string]string{"secretProviderClass": "provider1", csiPodName: "pod1", csiPodNamespace: "default"},
136-
},
137-
initObjects: []client.Object{
138-
&secretsstorev1.SecretProviderClass{
139-
ObjectMeta: metav1.ObjectMeta{
140-
Name: "provider1",
141-
Namespace: "default",
142-
},
143-
},
144-
},
154+
name: "provider not set in secret provider class",
155+
nodePublishVolReq: getRequest(t, func(*csi.NodePublishVolumeRequest) {}),
156+
initObjects: getInitObjects(func(s *secretsstorev1.SecretProviderClass) {
157+
s.Spec = secretsstorev1.SecretProviderClassSpec{}
158+
}),
145159
want: codes.Unknown,
146160
},
147161
{
148-
name: "parameters not set in secret provider class",
149-
nodePublishVolReq: &csi.NodePublishVolumeRequest{
150-
VolumeCapability: &csi.VolumeCapability{},
151-
VolumeId: "testvolid1",
152-
TargetPath: targetPath(t),
153-
VolumeContext: map[string]string{"secretProviderClass": "provider1", csiPodName: "pod1", csiPodNamespace: "default"},
154-
},
155-
initObjects: []client.Object{
156-
&secretsstorev1.SecretProviderClass{
157-
ObjectMeta: metav1.ObjectMeta{
158-
Name: "provider1",
159-
Namespace: "default",
160-
},
161-
Spec: secretsstorev1.SecretProviderClassSpec{
162-
Provider: "provider1",
163-
},
164-
},
165-
},
162+
name: "parameters not set in secret provider class",
163+
nodePublishVolReq: getRequest(t, func(*csi.NodePublishVolumeRequest) {}),
164+
initObjects: getInitObjects(func(s *secretsstorev1.SecretProviderClass) {
165+
s.Spec.Parameters = map[string]string{}
166+
}),
166167
want: codes.Unknown,
167168
},
168169
{
169170
name: "read only is not set to true",
170-
nodePublishVolReq: &csi.NodePublishVolumeRequest{
171-
VolumeCapability: &csi.VolumeCapability{},
172-
VolumeId: "testvolid1",
173-
TargetPath: targetPath(t),
174-
VolumeContext: map[string]string{"secretProviderClass": "provider1", csiPodName: "pod1", csiPodNamespace: "default"},
175-
},
176-
initObjects: []client.Object{
177-
&secretsstorev1.SecretProviderClass{
178-
ObjectMeta: metav1.ObjectMeta{
179-
Name: "provider1",
180-
Namespace: "default",
181-
},
182-
Spec: secretsstorev1.SecretProviderClassSpec{
183-
Provider: "provider1",
184-
Parameters: map[string]string{"parameter1": "value1"},
185-
},
186-
},
187-
},
188-
want: codes.InvalidArgument,
171+
nodePublishVolReq: getRequest(t, func(r *csi.NodePublishVolumeRequest) {
172+
r.Readonly = false
173+
}),
174+
initObjects: getInitObjects(func(*secretsstorev1.SecretProviderClass) {}),
175+
want: codes.InvalidArgument,
189176
},
190177
{
191-
name: "provider not installed",
192-
nodePublishVolReq: &csi.NodePublishVolumeRequest{
193-
VolumeCapability: &csi.VolumeCapability{},
194-
VolumeId: "testvolid1",
195-
TargetPath: targetPath(t),
196-
VolumeContext: map[string]string{
197-
"secretProviderClass": "provider1",
198-
csiPodName: "pod1",
199-
csiPodNamespace: "default",
200-
csiPodUID: "poduid1",
201-
},
202-
Readonly: true,
203-
},
204-
initObjects: []client.Object{
205-
&secretsstorev1.SecretProviderClass{
206-
ObjectMeta: metav1.ObjectMeta{
207-
Name: "provider1",
208-
Namespace: "default",
209-
},
210-
Spec: secretsstorev1.SecretProviderClassSpec{
211-
Provider: "provider_not_installed",
212-
Parameters: map[string]string{"parameter1": "value1"},
213-
},
214-
},
215-
},
178+
name: "provider not installed",
179+
nodePublishVolReq: getRequest(t, func(*csi.NodePublishVolumeRequest) {}),
180+
initObjects: getInitObjects(func(s *secretsstorev1.SecretProviderClass) {
181+
s.Spec.Provider = "provider_not_installed"
182+
}),
216183
want: codes.Unknown,
217184
},
185+
{
186+
name: "Invalid FSGroup",
187+
nodePublishVolReq: getRequest(t, func(r *csi.NodePublishVolumeRequest) {
188+
r.VolumeCapability = &csi.VolumeCapability{
189+
AccessType: &csi.VolumeCapability_Mount{
190+
Mount: &csi.VolumeCapability_MountVolume{
191+
VolumeMountGroup: "INVALID",
192+
},
193+
},
194+
}
195+
}),
196+
initObjects: getInitObjects(func(*secretsstorev1.SecretProviderClass) {}),
197+
want: codes.InvalidArgument,
198+
},
218199
}
219200

220201
s := scheme.Scheme
@@ -271,96 +252,48 @@ func TestNodePublishVolume(t *testing.T) {
271252
rotationConfig *rotationConfig
272253
}{
273254
{
274-
name: "volume mount",
275-
nodePublishVolReq: &csi.NodePublishVolumeRequest{
276-
VolumeCapability: &csi.VolumeCapability{},
277-
VolumeId: "testvolid1",
278-
TargetPath: targetPath(t),
279-
VolumeContext: map[string]string{
280-
"secretProviderClass": "provider1",
281-
csiPodName: "pod1",
282-
csiPodNamespace: "default",
283-
csiPodUID: "poduid1",
284-
},
285-
Readonly: true,
286-
},
287-
initObjects: []client.Object{
288-
&secretsstorev1.SecretProviderClass{
289-
ObjectMeta: metav1.ObjectMeta{
290-
Name: "provider1",
291-
Namespace: "default",
292-
},
293-
Spec: secretsstorev1.SecretProviderClassSpec{
294-
Provider: "provider1",
295-
Parameters: map[string]string{"parameter1": "value1"},
296-
},
297-
},
298-
},
255+
name: "volume mount",
256+
nodePublishVolReq: getRequest(t, func(*csi.NodePublishVolumeRequest) {}),
257+
initObjects: getInitObjects(func(*secretsstorev1.SecretProviderClass) {}),
299258
rotationConfig: &rotationConfig{
300259
enabled: false,
301260
rotationCacheDuration: time.Minute,
302261
},
303262
},
304263
{
305-
name: "volume mount with refresh token ",
306-
nodePublishVolReq: &csi.NodePublishVolumeRequest{
307-
VolumeCapability: &csi.VolumeCapability{},
308-
VolumeId: "testvolid1",
309-
TargetPath: targetPath(t),
310-
VolumeContext: map[string]string{
311-
"secretProviderClass": "provider1",
312-
csiPodName: "pod1",
313-
csiPodNamespace: "default",
314-
csiPodUID: "poduid1",
315-
// not a real token, just for testing
316-
csiPodServiceAccountTokens: `{"https://kubernetes.default.svc":{"token":"eyJhbGciOiJSUzI1NiIsImtpZCI6IjEyMyJ9.eyJhdWQiOlsiaHR0cHM6Ly9rdWJlcm5ldGVzLmRlZmF1bHQuc3ZjIl0sImV4cCI6MTYxMTk1OTM5NiwiaWF0IjoxNjExOTU4Nzk2LCJpc3MiOiJodHRwczovL2t1YmVybmV0ZXMuZGVmYXVsdC5zdmMiLCJrdWJlcm5ldGVzLmlvIjp7Im5hbWVzcGFjZSI6ImRlZmF1bHQiLCJzZXJ2aWNlYWNjb3VudCI6eyJuYW1lIjoiZGVmYXVsdCIsInVpZCI6IjA5MWUyNTU3LWJkODYtNDhhMC1iZmNmLWI1YTI4ZjRjODAyNCJ9fSwibmJmIjoxNjExOTU4Nzk2LCJzdWIiOiJzeXN0ZW06c2VydmljZWFjY291bnQ6ZGVmYXVsdDpkZWZhdWx0In0.YNU2Z_gEE84DGCt8lh9GuE8gmoof-Pk_7emp3fsyj9pq16DRiDaLtOdprH-njpOYqvtT5Uf_QspFc_RwD_pdq9UJWCeLxFkRTsYR5WSjhMFcl767c4Cwp_oZPYhaHd1x7aU1emH-9oarrM__tr1hSmGoAc2I0gUSkAYFueaTUSy5e5d9QKDfjVljDRc7Yrp6qAAfd1OuDdk1XYIjrqTHk1T1oqGGlcd3lRM_dKSsW5I_YqgKMrjwNt8yOKcdKBrgQhgC42GZbFDRVJDJHs_Hq32xo-2s3PJ8UZ_alN4wv8EbuwB987_FHBTc_XAULHPvp0mCv2C5h0V2A7gzccv30A","expirationTimestamp":"2021-01-29T22:29:56Z"}}`,
317-
"providerName": "provider1",
318-
},
319-
Readonly: true,
320-
},
321-
initObjects: []client.Object{
322-
&secretsstorev1.SecretProviderClass{
323-
ObjectMeta: metav1.ObjectMeta{
324-
Name: "provider1",
325-
Namespace: "default",
326-
},
327-
Spec: secretsstorev1.SecretProviderClassSpec{
328-
Provider: "provider1",
329-
Parameters: map[string]string{"parameter1": "value1"},
330-
},
331-
},
332-
},
264+
name: "volume mount with refresh token",
265+
nodePublishVolReq: getRequest(t, func(r *csi.NodePublishVolumeRequest) {
266+
// not a real token, just for testing
267+
r.VolumeContext[csiPodServiceAccountTokens] = `{"https://kubernetes.default.svc":{"token":"eyJhbGciOiJSUzI1NiIsImtpZCI6IjEyMyJ9.eyJhdWQiOlsiaHR0cHM6Ly9rdWJlcm5ldGVzLmRlZmF1bHQuc3ZjIl0sImV4cCI6MTYxMTk1OTM5NiwiaWF0IjoxNjExOTU4Nzk2LCJpc3MiOiJodHRwczovL2t1YmVybmV0ZXMuZGVmYXVsdC5zdmMiLCJrdWJlcm5ldGVzLmlvIjp7Im5hbWVzcGFjZSI6ImRlZmF1bHQiLCJzZXJ2aWNlYWNjb3VudCI6eyJuYW1lIjoiZGVmYXVsdCIsInVpZCI6IjA5MWUyNTU3LWJkODYtNDhhMC1iZmNmLWI1YTI4ZjRjODAyNCJ9fSwibmJmIjoxNjExOTU4Nzk2LCJzdWIiOiJzeXN0ZW06c2VydmljZWFjY291bnQ6ZGVmYXVsdDpkZWZhdWx0In0.YNU2Z_gEE84DGCt8lh9GuE8gmoof-Pk_7emp3fsyj9pq16DRiDaLtOdprH-njpOYqvtT5Uf_QspFc_RwD_pdq9UJWCeLxFkRTsYR5WSjhMFcl767c4Cwp_oZPYhaHd1x7aU1emH-9oarrM__tr1hSmGoAc2I0gUSkAYFueaTUSy5e5d9QKDfjVljDRc7Yrp6qAAfd1OuDdk1XYIjrqTHk1T1oqGGlcd3lRM_dKSsW5I_YqgKMrjwNt8yOKcdKBrgQhgC42GZbFDRVJDJHs_Hq32xo-2s3PJ8UZ_alN4wv8EbuwB987_FHBTc_XAULHPvp0mCv2C5h0V2A7gzccv30A","expirationTimestamp":"2021-01-29T22:29:56Z"}}`
268+
r.VolumeContext["providerName"] = "provider1"
269+
}),
270+
initObjects: getInitObjects(func(*secretsstorev1.SecretProviderClass) {}),
333271
rotationConfig: &rotationConfig{
334272
enabled: true,
335273
rotationCacheDuration: -1 * time.Minute, // Using negative interval to pass the rotation interval check in unit tests
336274
},
337275
},
338276
{
339-
name: "volume mount with rotation but skipped",
340-
nodePublishVolReq: &csi.NodePublishVolumeRequest{
341-
VolumeCapability: &csi.VolumeCapability{},
342-
VolumeId: "testvolid1",
343-
TargetPath: targetPath(t),
344-
VolumeContext: map[string]string{
345-
"secretProviderClass": "provider1",
346-
csiPodName: "pod1",
347-
csiPodNamespace: "default",
348-
csiPodUID: "poduid1",
349-
},
350-
Readonly: true,
277+
name: "volume mount with rotation but skipped",
278+
nodePublishVolReq: getRequest(t, func(*csi.NodePublishVolumeRequest) {}),
279+
initObjects: getInitObjects(func(*secretsstorev1.SecretProviderClass) {}),
280+
rotationConfig: &rotationConfig{
281+
enabled: true,
282+
rotationCacheDuration: time.Minute,
351283
},
352-
initObjects: []client.Object{
353-
&secretsstorev1.SecretProviderClass{
354-
ObjectMeta: metav1.ObjectMeta{
355-
Name: "provider1",
356-
Namespace: "default",
357-
},
358-
Spec: secretsstorev1.SecretProviderClassSpec{
359-
Provider: "provider1",
360-
Parameters: map[string]string{"parameter1": "value1"},
284+
},
285+
{
286+
name: "volume mount with valid FSGroup",
287+
nodePublishVolReq: getRequest(t, func(r *csi.NodePublishVolumeRequest) {
288+
r.VolumeCapability = &csi.VolumeCapability{
289+
AccessType: &csi.VolumeCapability_Mount{
290+
Mount: &csi.VolumeCapability_MountVolume{
291+
VolumeMountGroup: "1004",
292+
},
361293
},
362-
},
363-
},
294+
}
295+
}),
296+
initObjects: getInitObjects(func(*secretsstorev1.SecretProviderClass) {}),
364297
rotationConfig: &rotationConfig{
365298
enabled: true,
366299
rotationCacheDuration: time.Minute,

0 commit comments

Comments
 (0)