Skip to content

Commit fec20e4

Browse files
inteoninteon
committed
add fsGroup support
Signed-off-by: Tim Ramlot <[email protected]>
1 parent 4e16ea6 commit fec20e4

File tree

6 files changed

+68
-29
lines changed

6 files changed

+68
-29
lines changed

‎driver/controllerserver.go

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -70,15 +70,7 @@ func (cs *controllerServer) GetCapacity(ctx context.Context, req *csi.GetCapacit
7070
// Default supports all capabilities
7171
func (cs *controllerServer) ControllerGetCapabilities(ctx context.Context, req *csi.ControllerGetCapabilitiesRequest) (*csi.ControllerGetCapabilitiesResponse, error) {
7272
return &csi.ControllerGetCapabilitiesResponse{
73-
Capabilities: []*csi.ControllerServiceCapability{
74-
{
75-
Type: &csi.ControllerServiceCapability_Rpc{
76-
Rpc: &csi.ControllerServiceCapability_RPC{
77-
Type: csi.ControllerServiceCapability_RPC_UNKNOWN,
78-
},
79-
},
80-
},
81-
},
73+
Capabilities: []*csi.ControllerServiceCapability{},
8274
}, nil
8375
}
8476

‎driver/nodeserver.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -198,7 +198,7 @@ func (ns *nodeServer) NodeGetCapabilities(ctx context.Context, request *csi.Node
198198
{
199199
Type: &csi.NodeServiceCapability_Rpc{
200200
Rpc: &csi.NodeServiceCapability_RPC{
201-
Type: csi.NodeServiceCapability_RPC_UNKNOWN,
201+
Type: csi.NodeServiceCapability_RPC_VOLUME_MOUNT_GROUP,
202202
},
203203
},
204204
},

‎driver/server.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,7 @@ func parseEndpoint(ep string) (string, string, error) {
9696
return s[0], s[1], nil
9797
}
9898
}
99-
return "", "", fmt.Errorf("Invalid endpoint: %v", ep)
99+
return "", "", fmt.Errorf("invalid endpoint: %v", ep)
100100
}
101101

102102
func loggingInterceptor(log logr.Logger) grpc.UnaryServerInterceptor {

‎metadata/metadata.go

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -38,14 +38,18 @@ type Metadata struct {
3838
// System-specific attributes extracted from the NodePublishVolume request.
3939
// These are sourced from the VolumeContext.
4040
VolumeContext map[string]string `json:"volumeContext,omitempty"`
41+
42+
// VolumeMountGroup is the filesystem group that the volume should be mounted as.
43+
VolumeMountGroup string `json:"volumeMountGroup,omitempty"`
4144
}
4245

4346
// FromNodePublishVolumeRequest constructs a Metadata from a NodePublishVolumeRequest.
4447
// The NextIssuanceTime field will NOT be set.
4548
func FromNodePublishVolumeRequest(request *csi.NodePublishVolumeRequest) Metadata {
4649
return Metadata{
47-
VolumeID: request.GetVolumeId(),
48-
TargetPath: request.GetTargetPath(),
49-
VolumeContext: request.GetVolumeContext(),
50+
VolumeID: request.GetVolumeId(),
51+
TargetPath: request.GetTargetPath(),
52+
VolumeContext: request.GetVolumeContext(),
53+
VolumeMountGroup: request.GetVolumeCapability().GetMount().GetVolumeMountGroup(),
5054
}
5155
}

‎storage/filesystem.go

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -333,14 +333,20 @@ func (f *Filesystem) fsGroupForMetadata(meta metadata.Metadata) (*int64, error)
333333
return f.FixedFSGroup, nil
334334
}
335335

336+
// The VolumeAttribute takes precedence over the VolumeMountGroup that is
337+
// set using the securityContext.fsGroup field. This way, we can support more
338+
// granular control over the fsGroup per volume.
339+
fsGroupStr := ""
336340
// If the FSGroupVolumeAttributeKey is not defined, no ownership can change.
337-
if len(f.FSGroupVolumeAttributeKey) == 0 {
338-
return nil, nil
341+
if fsGroupStr == "" && len(f.FSGroupVolumeAttributeKey) > 0 {
342+
fsGroupStr = meta.VolumeContext[f.FSGroupVolumeAttributeKey]
343+
}
344+
if fsGroupStr == "" {
345+
fsGroupStr = meta.VolumeMountGroup
339346
}
340347

341-
fsGroupStr, ok := meta.VolumeContext[f.FSGroupVolumeAttributeKey]
342-
if !ok {
343-
// If the attribute has not been set, return no ownership change.
348+
// If the attribute has not been set, return no ownership change.
349+
if fsGroupStr == "" {
344350
return nil, nil
345351
}
346352

‎storage/filesystem_test.go

Lines changed: 47 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -144,80 +144,116 @@ func Test_fsGroupForMetadata(t *testing.T) {
144144

145145
tests := map[string]struct {
146146
fixedFSGroup *int64
147+
metaVolumeMountGroup string
147148
fsGroupVolumeAttributeKey string
148149
volumeContext map[string]string
149150

150151
expGID *int64
151152
expErr bool
152153
}{
153-
"FixedFSGroup=nil FSGroupVolumeAttributeKey='', should return nil gid": {
154+
"FixedFSGroup=nil meta.VolumeMountGroup='' FSGroupVolumeAttributeKey='', should return nil gid": {
154155
fixedFSGroup: nil,
156+
metaVolumeMountGroup: "",
155157
fsGroupVolumeAttributeKey: "",
156158
volumeContext: map[string]string{},
157159
expGID: nil,
158160
expErr: false,
159161
},
160-
"FixedFSGroup=10 FSGroupVolumeAttributeKey='', should return 10": {
162+
"FixedFSGroup=10 meta.VolumeMountGroup='' FSGroupVolumeAttributeKey='', should return 10": {
161163
fixedFSGroup: intPtr(10),
164+
metaVolumeMountGroup: "",
162165
fsGroupVolumeAttributeKey: "",
163166
volumeContext: map[string]string{},
164167
expGID: intPtr(10),
165168
expErr: false,
166169
},
167-
"FixedFSGroup=nil FSGroupVolumeAttributeKey=defined but not present in context, should return nil": {
170+
"FixedFSGroup=nil meta.VolumeMountGroup='70' FSGroupVolumeAttributeKey='', should return 70": {
168171
fixedFSGroup: nil,
172+
metaVolumeMountGroup: "70",
173+
fsGroupVolumeAttributeKey: "",
174+
volumeContext: map[string]string{},
175+
expGID: intPtr(70),
176+
expErr: false,
177+
},
178+
"FixedFSGroup=nil meta.VolumeMountGroup='' FSGroupVolumeAttributeKey=defined but not present in context, should return nil": {
179+
fixedFSGroup: nil,
180+
metaVolumeMountGroup: "",
169181
fsGroupVolumeAttributeKey: "fs-gid",
170182
volumeContext: map[string]string{},
171183
expGID: nil,
172184
expErr: false,
173185
},
174-
"FixedFSGroup=nil FSGroupVolumeAttributeKey=defined and present in context, should return 20": {
186+
"FixedFSGroup=nil meta.VolumeMountGroup='70' FSGroupVolumeAttributeKey=defined but not present in context, should return 70": {
187+
fixedFSGroup: nil,
188+
metaVolumeMountGroup: "70",
189+
fsGroupVolumeAttributeKey: "fs-gid",
190+
volumeContext: map[string]string{},
191+
expGID: intPtr(70),
192+
expErr: false,
193+
},
194+
"FixedFSGroup=nil meta.VolumeMountGroup='' FSGroupVolumeAttributeKey=defined and present in context, should return 20": {
195+
fixedFSGroup: nil,
196+
metaVolumeMountGroup: "",
197+
fsGroupVolumeAttributeKey: "fs-gid",
198+
volumeContext: map[string]string{
199+
"fs-gid": "20",
200+
},
201+
expGID: intPtr(20),
202+
expErr: false,
203+
},
204+
"FixedFSGroup=nil meta.VolumeMountGroup='10' FSGroupVolumeAttributeKey=defined and present in context, should return 20": {
175205
fixedFSGroup: nil,
206+
metaVolumeMountGroup: "10",
176207
fsGroupVolumeAttributeKey: "fs-gid",
177208
volumeContext: map[string]string{
178209
"fs-gid": "20",
179210
},
180211
expGID: intPtr(20),
181212
expErr: false,
182213
},
183-
"FixedFSGroup=nil FSGroupVolumeAttributeKey=defined and present in context but value of 0, should error": {
214+
"FixedFSGroup=nil meta.VolumeMountGroup='' FSGroupVolumeAttributeKey=defined and present in context but value of 0, should error": {
184215
fixedFSGroup: nil,
216+
metaVolumeMountGroup: "",
185217
fsGroupVolumeAttributeKey: "fs-gid",
186218
volumeContext: map[string]string{
187219
"fs-gid": "0",
188220
},
189221
expGID: nil,
190222
expErr: true,
191223
},
192-
"FixedFSGroup=nil FSGroupVolumeAttributeKey=defined and present in context but value of -1, should error": {
224+
"FixedFSGroup=nil meta.VolumeMountGroup='' FSGroupVolumeAttributeKey=defined and present in context but value of -1, should error": {
193225
fixedFSGroup: nil,
226+
metaVolumeMountGroup: "",
194227
fsGroupVolumeAttributeKey: "fs-gid",
195228
volumeContext: map[string]string{
196229
"fs-gid": "-1",
197230
},
198231
expGID: nil,
199232
expErr: true,
200233
},
201-
"FixedFSGroup=nil FSGroupVolumeAttributeKey=defined and present in context but value greater than the max gid, should error": {
234+
"FixedFSGroup=nil meta.VolumeMountGroup='' FSGroupVolumeAttributeKey=defined and present in context but value greater than the max gid, should error": {
202235
fixedFSGroup: nil,
236+
metaVolumeMountGroup: "",
203237
fsGroupVolumeAttributeKey: "fs-gid",
204238
volumeContext: map[string]string{
205239
"fs-gid": "4294967296",
206240
},
207241
expGID: nil,
208242
expErr: true,
209243
},
210-
"FixedFSGroup=nil FSGroupVolumeAttributeKey=defined and present in context but with bad value, should return error": {
244+
"FixedFSGroup=nil meta.VolumeMountGroup='' FSGroupVolumeAttributeKey=defined and present in context but with bad value, should return error": {
211245
fixedFSGroup: nil,
246+
metaVolumeMountGroup: "",
212247
fsGroupVolumeAttributeKey: "fs-gid",
213248
volumeContext: map[string]string{
214249
"fs-gid": "bad-value",
215250
},
216251
expGID: nil,
217252
expErr: true,
218253
},
219-
"FixedFSGroup=10 FSGroupVolumeAttributeKey=defined and present in context, should return superseding FixedFSGroup (10)": {
254+
"FixedFSGroup=10 meta.VolumeMountGroup='' FSGroupVolumeAttributeKey=defined and present in context, should return superseding FixedFSGroup (10)": {
220255
fixedFSGroup: intPtr(10),
256+
metaVolumeMountGroup: "",
221257
fsGroupVolumeAttributeKey: "fs-gid",
222258
volumeContext: map[string]string{
223259
"fs-gid": "20",
@@ -235,7 +271,8 @@ func Test_fsGroupForMetadata(t *testing.T) {
235271
}
236272

237273
gid, err := f.fsGroupForMetadata(metadata.Metadata{
238-
VolumeContext: test.volumeContext,
274+
VolumeContext: test.volumeContext,
275+
VolumeMountGroup: test.metaVolumeMountGroup,
239276
})
240277

241278
if (err != nil) != test.expErr {

0 commit comments

Comments
 (0)