Skip to content

Commit

Permalink
csi: check for empty arguments on CSI endpoint (#8027)
Browse files Browse the repository at this point in the history
Some of the CSI RPC endpoints were missing validation that the ID or
the Volume definition was present. This could result in nonsense
`CSIVolume` structs being written to raft during registration. This
changeset corrects that bug and adds validation checks to present
nicer error messages to operators in some other cases.
  • Loading branch information
tgross committed May 27, 2020
1 parent 4e365e0 commit 72f500a
Showing 1 changed file with 24 additions and 0 deletions.
24 changes: 24 additions & 0 deletions nomad/csi_endpoint.go
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,10 @@ func (v *CSIVolume) Get(args *structs.CSIVolumeGetRequest, reply *structs.CSIVol
metricsStart := time.Now()
defer metrics.MeasureSince([]string{"nomad", "volume", "get"}, metricsStart)

if args.ID == "" {
return fmt.Errorf("missing volume ID")
}

opts := blockingOptions{
queryOpts: &args.QueryOptions,
queryMeta: &reply.QueryMeta,
Expand Down Expand Up @@ -266,6 +270,10 @@ func (v *CSIVolume) Register(args *structs.CSIVolumeRegisterRequest, reply *stru
return structs.ErrPermissionDenied
}

if args.Volumes == nil || len(args.Volumes) == 0 {
return fmt.Errorf("missing volume definition")
}

// This is the only namespace we ACL checked, force all the volumes to use it.
// We also validate that the plugin exists for each plugin, and validate the
// capabilities when the plugin has a controller.
Expand Down Expand Up @@ -318,6 +326,10 @@ func (v *CSIVolume) Deregister(args *structs.CSIVolumeDeregisterRequest, reply *
return structs.ErrPermissionDenied
}

if len(args.VolumeIDs) == 0 {
return fmt.Errorf("missing volume IDs")
}

resp, index, err := v.srv.raftApply(structs.CSIVolumeDeregisterRequestType, args)
if err != nil {
v.logger.Error("csi raft apply failed", "error", err, "method", "deregister")
Expand Down Expand Up @@ -351,6 +363,10 @@ func (v *CSIVolume) Claim(args *structs.CSIVolumeClaimRequest, reply *structs.CS
return structs.ErrPermissionDenied
}

if args.VolumeID == "" {
return fmt.Errorf("missing volume ID")
}

// COMPAT(1.0): the NodeID field was added after 0.11.0 and so we
// need to ensure it's been populated during upgrades from 0.11.0
// to later patch versions. Remove this block in 1.0
Expand Down Expand Up @@ -564,6 +580,10 @@ func (v *CSIPlugin) Get(args *structs.CSIPluginGetRequest, reply *structs.CSIPlu
metricsStart := time.Now()
defer metrics.MeasureSince([]string{"nomad", "plugin", "get"}, metricsStart)

if args.ID == "" {
return fmt.Errorf("missing plugin ID")
}

opts := blockingOptions{
queryOpts: &args.QueryOptions,
queryMeta: &reply.QueryMeta,
Expand Down Expand Up @@ -616,6 +636,10 @@ func (v *CSIPlugin) Delete(args *structs.CSIPluginDeleteRequest, reply *structs.
metricsStart := time.Now()
defer metrics.MeasureSince([]string{"nomad", "plugin", "delete"}, metricsStart)

if args.ID == "" {
return fmt.Errorf("missing plugin ID")
}

resp, index, err := v.srv.raftApply(structs.CSIPluginDeleteRequestType, args)
if err != nil {
v.logger.Error("csi raft apply failed", "error", err, "method", "delete")
Expand Down

0 comments on commit 72f500a

Please sign in to comment.