Skip to content

Commit 084acbb

Browse files
cert-manager-prow[bot]cert-manager-prow[bot]
authored
Merge pull request #72 from inteon/fsgroup
Add securityContext.fsGroup support, remove FixedFSGroup field
2 parents 4e16ea6 + 449d72f commit 084acbb

File tree

7 files changed

+60
-131
lines changed

7 files changed

+60
-131
lines changed

‎driver/controllerserver.go

Lines changed: 2 additions & 65 deletions
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,6 @@ import (
2020
"context"
2121

2222
"github.com/container-storage-interface/spec/lib/go/csi"
23-
"google.golang.org/grpc/codes"
24-
"google.golang.org/grpc/status"
2523
)
2624

2725
var _ csi.ControllerServer = &controllerServer{} // compiler validation
@@ -30,70 +28,9 @@ type controllerServer struct {
3028
csi.UnimplementedControllerServer
3129
}
3230

33-
func (cs *controllerServer) ControllerModifyVolume(ctx context.Context, request *csi.ControllerModifyVolumeRequest) (*csi.ControllerModifyVolumeResponse, error) {
34-
return nil, status.Error(codes.Unimplemented, "ControllerModifyVolume not implemented")
35-
}
36-
37-
func (cs *controllerServer) ControllerGetVolume(ctx context.Context, request *csi.ControllerGetVolumeRequest) (*csi.ControllerGetVolumeResponse, error) {
38-
return nil, status.Error(codes.Unimplemented, "ControllerGetVolume not implemented")
39-
}
40-
41-
func (cs *controllerServer) CreateVolume(ctx context.Context, req *csi.CreateVolumeRequest) (*csi.CreateVolumeResponse, error) {
42-
return nil, status.Error(codes.Unimplemented, "CreateVolume not implemented")
43-
}
44-
45-
func (cs *controllerServer) DeleteVolume(ctx context.Context, req *csi.DeleteVolumeRequest) (*csi.DeleteVolumeResponse, error) {
46-
return nil, status.Error(codes.Unimplemented, "DeleteVolume not implemented")
47-
}
48-
49-
func (cs *controllerServer) ControllerPublishVolume(ctx context.Context, req *csi.ControllerPublishVolumeRequest) (*csi.ControllerPublishVolumeResponse, error) {
50-
return nil, status.Error(codes.Unimplemented, "ControllerPublishVolume not implemented")
51-
}
52-
53-
func (cs *controllerServer) ControllerUnpublishVolume(ctx context.Context, req *csi.ControllerUnpublishVolumeRequest) (*csi.ControllerUnpublishVolumeResponse, error) {
54-
return nil, status.Error(codes.Unimplemented, "ControllerUnpublishVolume not implemented")
55-
}
56-
57-
func (cs *controllerServer) ValidateVolumeCapabilities(ctx context.Context, req *csi.ValidateVolumeCapabilitiesRequest) (*csi.ValidateVolumeCapabilitiesResponse, error) {
58-
return nil, status.Error(codes.Unimplemented, "ValidateVolumeCapabilities not implemented")
59-
}
60-
61-
func (cs *controllerServer) ListVolumes(ctx context.Context, req *csi.ListVolumesRequest) (*csi.ListVolumesResponse, error) {
62-
return nil, status.Error(codes.Unimplemented, "ListVolumes not implemented")
63-
}
64-
65-
func (cs *controllerServer) GetCapacity(ctx context.Context, req *csi.GetCapacityRequest) (*csi.GetCapacityResponse, error) {
66-
return nil, status.Error(codes.Unimplemented, "GetCapacity not implemented")
67-
}
68-
69-
// ControllerGetCapabilities implements the default GRPC callout.
70-
// Default supports all capabilities
31+
// Advertise no capabilities.
7132
func (cs *controllerServer) ControllerGetCapabilities(ctx context.Context, req *csi.ControllerGetCapabilitiesRequest) (*csi.ControllerGetCapabilitiesResponse, error) {
7233
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-
},
34+
Capabilities: []*csi.ControllerServiceCapability{},
8235
}, nil
8336
}
84-
85-
func (cs *controllerServer) CreateSnapshot(ctx context.Context, req *csi.CreateSnapshotRequest) (*csi.CreateSnapshotResponse, error) {
86-
return nil, status.Error(codes.Unimplemented, "CreateSnapshot not implemented")
87-
}
88-
89-
func (cs *controllerServer) DeleteSnapshot(ctx context.Context, req *csi.DeleteSnapshotRequest) (*csi.DeleteSnapshotResponse, error) {
90-
return nil, status.Error(codes.Unimplemented, "DeleteSnapshot not implemented")
91-
}
92-
93-
func (cs *controllerServer) ListSnapshots(ctx context.Context, req *csi.ListSnapshotsRequest) (*csi.ListSnapshotsResponse, error) {
94-
return nil, status.Error(codes.Unimplemented, "ListSnapshots not implemented")
95-
}
96-
97-
func (cs *controllerServer) ControllerExpandVolume(ctx context.Context, req *csi.ControllerExpandVolumeRequest) (*csi.ControllerExpandVolumeResponse, error) {
98-
return nil, status.Error(codes.Unimplemented, "ControllerExpandVolume not implemented")
99-
}

‎driver/identityserver.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,7 @@ func (ids *identityServer) Probe(ctx context.Context, req *csi.ProbeRequest) (*c
5858
return &csi.ProbeResponse{}, nil
5959
}
6060

61+
// Advertise the CONTROLLER_SERVICE capability, indicating the controller service can be called.
6162
func (ids *identityServer) GetPluginCapabilities(ctx context.Context, req *csi.GetPluginCapabilitiesRequest) (*csi.GetPluginCapabilitiesResponse, error) {
6263
return &csi.GetPluginCapabilitiesResponse{
6364
Capabilities: []*csi.PluginCapability{

‎driver/nodeserver.go

Lines changed: 2 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -149,14 +149,6 @@ func loggerForMetadata(log logr.Logger, meta metadata.Metadata) logr.Logger {
149149
return log.WithValues("pod_name", meta.VolumeContext["csi.storage.k8s.io/pod.name"])
150150
}
151151

152-
func (ns *nodeServer) NodeStageVolume(ctx context.Context, request *csi.NodeStageVolumeRequest) (*csi.NodeStageVolumeResponse, error) {
153-
return nil, status.Error(codes.Unimplemented, "NodeStageVolume not implemented")
154-
}
155-
156-
func (ns *nodeServer) NodeUnstageVolume(ctx context.Context, request *csi.NodeUnstageVolumeRequest) (*csi.NodeUnstageVolumeResponse, error) {
157-
return nil, status.Error(codes.Unimplemented, "NodeUnstageVolume not implemented")
158-
}
159-
160152
func (ns *nodeServer) NodeUnpublishVolume(ctx context.Context, request *csi.NodeUnpublishVolumeRequest) (*csi.NodeUnpublishVolumeResponse, error) {
161153
log := ns.log.WithValues("volume_id", request.VolumeId, "target_path", request.TargetPath)
162154
ns.manager.UnmanageVolume(request.GetVolumeId())
@@ -184,21 +176,14 @@ func (ns *nodeServer) NodeUnpublishVolume(ctx context.Context, request *csi.Node
184176
return &csi.NodeUnpublishVolumeResponse{}, nil
185177
}
186178

187-
func (ns *nodeServer) NodeGetVolumeStats(ctx context.Context, request *csi.NodeGetVolumeStatsRequest) (*csi.NodeGetVolumeStatsResponse, error) {
188-
return nil, status.Error(codes.Unimplemented, "NodeGetVolumeStats not implemented")
189-
}
190-
191-
func (ns *nodeServer) NodeExpandVolume(ctx context.Context, request *csi.NodeExpandVolumeRequest) (*csi.NodeExpandVolumeResponse, error) {
192-
return nil, status.Error(codes.Unimplemented, "NodeExpandVolume not implemented")
193-
}
194-
179+
// Advertise the VOLUME_MOUNT_GROUP capability, indicating that we can handle the securityContext.fsGroup option.
195180
func (ns *nodeServer) NodeGetCapabilities(ctx context.Context, request *csi.NodeGetCapabilitiesRequest) (*csi.NodeGetCapabilitiesResponse, error) {
196181
return &csi.NodeGetCapabilitiesResponse{
197182
Capabilities: []*csi.NodeServiceCapability{
198183
{
199184
Type: &csi.NodeServiceCapability_Rpc{
200185
Rpc: &csi.NodeServiceCapability_RPC{
201-
Type: csi.NodeServiceCapability_RPC_UNKNOWN,
186+
Type: csi.NodeServiceCapability_RPC_VOLUME_MOUNT_GROUP,
202187
},
203188
},
204189
},

‎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 & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -49,16 +49,10 @@ type Filesystem struct {
4949
// used by the 'read only' methods
5050
fs fs.StatFS
5151

52-
// FixedFSGroup is an optional field which will set the gid ownership of all
53-
// volume's data directories to this value.
54-
// If this value is set, FSGroupVolumeAttributeKey has no effect.
55-
FixedFSGroup *int64
56-
5752
// FSGroupVolumeAttributeKey is an optional well-known key in the volume
5853
// attributes. If this attribute is present in the context when writing
5954
// files, gid ownership of the volume's data directory will be changed to
6055
// the value. Attribute value must be a valid int64 value.
61-
// If FixedFSGroup is defined, this field has no effect.
6256
FSGroupVolumeAttributeKey string
6357
}
6458

@@ -328,19 +322,20 @@ func makePayload(in map[string][]byte) map[string]util.FileProjection {
328322
// directory should be changed to. Returns nil if ownership should not be
329323
// changed.
330324
func (f *Filesystem) fsGroupForMetadata(meta metadata.Metadata) (*int64, error) {
331-
// FixedFSGroup takes precedence over attribute key.
332-
if f.FixedFSGroup != nil {
333-
return f.FixedFSGroup, nil
334-
}
335-
325+
// The VolumeAttribute takes precedence over the VolumeMountGroup that is
326+
// set using the securityContext.fsGroup field. This way, we can support more
327+
// granular control over the fsGroup per volume.
328+
fsGroupStr := ""
336329
// If the FSGroupVolumeAttributeKey is not defined, no ownership can change.
337-
if len(f.FSGroupVolumeAttributeKey) == 0 {
338-
return nil, nil
330+
if fsGroupStr == "" && len(f.FSGroupVolumeAttributeKey) > 0 {
331+
fsGroupStr = meta.VolumeContext[f.FSGroupVolumeAttributeKey]
332+
}
333+
if fsGroupStr == "" {
334+
fsGroupStr = meta.VolumeMountGroup
339335
}
340336

341-
fsGroupStr, ok := meta.VolumeContext[f.FSGroupVolumeAttributeKey]
342-
if !ok {
343-
// If the attribute has not been set, return no ownership change.
337+
// If the attribute has not been set, return no ownership change.
338+
if fsGroupStr == "" {
344339
return nil, nil
345340
}
346341

‎storage/filesystem_test.go

Lines changed: 36 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -143,99 +143,106 @@ func Test_fsGroupForMetadata(t *testing.T) {
143143
}
144144

145145
tests := map[string]struct {
146-
fixedFSGroup *int64
146+
metaVolumeMountGroup string
147147
fsGroupVolumeAttributeKey string
148148
volumeContext map[string]string
149149

150150
expGID *int64
151151
expErr bool
152152
}{
153-
"FixedFSGroup=nil FSGroupVolumeAttributeKey='', should return nil gid": {
154-
fixedFSGroup: nil,
153+
"meta.VolumeMountGroup='' FSGroupVolumeAttributeKey='', should return nil gid": {
154+
metaVolumeMountGroup: "",
155155
fsGroupVolumeAttributeKey: "",
156156
volumeContext: map[string]string{},
157157
expGID: nil,
158158
expErr: false,
159159
},
160-
"FixedFSGroup=10 FSGroupVolumeAttributeKey='', should return 10": {
161-
fixedFSGroup: intPtr(10),
160+
"meta.VolumeMountGroup='70' FSGroupVolumeAttributeKey='', should return 70": {
161+
metaVolumeMountGroup: "70",
162162
fsGroupVolumeAttributeKey: "",
163163
volumeContext: map[string]string{},
164-
expGID: intPtr(10),
164+
expGID: intPtr(70),
165165
expErr: false,
166166
},
167-
"FixedFSGroup=nil FSGroupVolumeAttributeKey=defined but not present in context, should return nil": {
168-
fixedFSGroup: nil,
167+
"meta.VolumeMountGroup='' FSGroupVolumeAttributeKey=defined but not present in context, should return nil": {
168+
metaVolumeMountGroup: "",
169169
fsGroupVolumeAttributeKey: "fs-gid",
170170
volumeContext: map[string]string{},
171171
expGID: nil,
172172
expErr: false,
173173
},
174-
"FixedFSGroup=nil FSGroupVolumeAttributeKey=defined and present in context, should return 20": {
175-
fixedFSGroup: nil,
174+
"meta.VolumeMountGroup='70' FSGroupVolumeAttributeKey=defined but not present in context, should return 70": {
175+
metaVolumeMountGroup: "70",
176+
fsGroupVolumeAttributeKey: "fs-gid",
177+
volumeContext: map[string]string{},
178+
expGID: intPtr(70),
179+
expErr: false,
180+
},
181+
"meta.VolumeMountGroup='' FSGroupVolumeAttributeKey=defined and present in context, should return 20": {
182+
metaVolumeMountGroup: "",
176183
fsGroupVolumeAttributeKey: "fs-gid",
177184
volumeContext: map[string]string{
178185
"fs-gid": "20",
179186
},
180187
expGID: intPtr(20),
181188
expErr: false,
182189
},
183-
"FixedFSGroup=nil FSGroupVolumeAttributeKey=defined and present in context but value of 0, should error": {
184-
fixedFSGroup: nil,
190+
"meta.VolumeMountGroup='10' FSGroupVolumeAttributeKey=defined and present in context, should return 20": {
191+
metaVolumeMountGroup: "10",
192+
fsGroupVolumeAttributeKey: "fs-gid",
193+
volumeContext: map[string]string{
194+
"fs-gid": "20",
195+
},
196+
expGID: intPtr(20),
197+
expErr: false,
198+
},
199+
"meta.VolumeMountGroup='' FSGroupVolumeAttributeKey=defined and present in context but value of 0, should error": {
200+
metaVolumeMountGroup: "",
185201
fsGroupVolumeAttributeKey: "fs-gid",
186202
volumeContext: map[string]string{
187203
"fs-gid": "0",
188204
},
189205
expGID: nil,
190206
expErr: true,
191207
},
192-
"FixedFSGroup=nil FSGroupVolumeAttributeKey=defined and present in context but value of -1, should error": {
193-
fixedFSGroup: nil,
208+
"meta.VolumeMountGroup='' FSGroupVolumeAttributeKey=defined and present in context but value of -1, should error": {
209+
metaVolumeMountGroup: "",
194210
fsGroupVolumeAttributeKey: "fs-gid",
195211
volumeContext: map[string]string{
196212
"fs-gid": "-1",
197213
},
198214
expGID: nil,
199215
expErr: true,
200216
},
201-
"FixedFSGroup=nil FSGroupVolumeAttributeKey=defined and present in context but value greater than the max gid, should error": {
202-
fixedFSGroup: nil,
217+
"meta.VolumeMountGroup='' FSGroupVolumeAttributeKey=defined and present in context but value greater than the max gid, should error": {
218+
metaVolumeMountGroup: "",
203219
fsGroupVolumeAttributeKey: "fs-gid",
204220
volumeContext: map[string]string{
205221
"fs-gid": "4294967296",
206222
},
207223
expGID: nil,
208224
expErr: true,
209225
},
210-
"FixedFSGroup=nil FSGroupVolumeAttributeKey=defined and present in context but with bad value, should return error": {
211-
fixedFSGroup: nil,
226+
"meta.VolumeMountGroup='' FSGroupVolumeAttributeKey=defined and present in context but with bad value, should return error": {
227+
metaVolumeMountGroup: "",
212228
fsGroupVolumeAttributeKey: "fs-gid",
213229
volumeContext: map[string]string{
214230
"fs-gid": "bad-value",
215231
},
216232
expGID: nil,
217233
expErr: true,
218234
},
219-
"FixedFSGroup=10 FSGroupVolumeAttributeKey=defined and present in context, should return superseding FixedFSGroup (10)": {
220-
fixedFSGroup: intPtr(10),
221-
fsGroupVolumeAttributeKey: "fs-gid",
222-
volumeContext: map[string]string{
223-
"fs-gid": "20",
224-
},
225-
expGID: intPtr(10),
226-
expErr: false,
227-
},
228235
}
229236

230237
for name, test := range tests {
231238
t.Run(name, func(t *testing.T) {
232239
f := Filesystem{
233-
FixedFSGroup: test.fixedFSGroup,
234240
FSGroupVolumeAttributeKey: test.fsGroupVolumeAttributeKey,
235241
}
236242

237243
gid, err := f.fsGroupForMetadata(metadata.Metadata{
238-
VolumeContext: test.volumeContext,
244+
VolumeContext: test.volumeContext,
245+
VolumeMountGroup: test.metaVolumeMountGroup,
239246
})
240247

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

0 commit comments

Comments
 (0)