Support EBS Task Attach non-root user mode #4757
Conversation
agent/api/task/task_linux.go
Outdated
| if ebsConfig, ok := vol.(*taskresourcevolume.EBSTaskVolumeConfig); ok { | ||
| // This container mounts an EBS volume, generate GID from SourceVolumeHostPath | ||
| gid := utils.GenerateGIDFromPath(ebsConfig.SourceVolumeHostPath) | ||
| logger.Info("gid from task linux go " + strconv.Itoa(gid)) |
There was a problem hiding this comment.
| logger.Info("gid from task linux go " + strconv.Itoa(gid)) | |
| logger.Debug("Generated a GID from EBS volume's host path", logger.Fields{field.TaskID: task.GetID(), "EBS SourceVolumeHostPath": ebsConfig.SourceVolumeHostPath, "GID": strconv(Itoa(gid))) |
| @@ -266,3 +267,8 @@ func (task *Task) BuildCNIConfigAwsvpc(includeIPAMConfig bool, cniConfig *ecscni | |||
| func (task *Task) BuildCNIConfigBridgeMode(cniConfig *ecscni.Config, containerName string) (*ecscni.Config, error) { | |||
| return nil, errors.New("unsupported platform") | |||
| } | |||
|
|
|||
| // Not Supported for Windows | |||
| func (task *Task) getSupplementaryGroups(container *apicontainer.Container) []string { | |||
There was a problem hiding this comment.
Is the EBS Task Attach feature as a whole supported for Windows?
There was a problem hiding this comment.
agent/app/agent_capability.go
Outdated
| @@ -83,6 +83,7 @@ const ( | |||
| capabilityServiceConnect = "service-connect-v1" | |||
| capabilityGpuDriverVersion = "gpu-driver-version" | |||
| capabilityEBSTaskAttach = "storage.ebs-task-volume-attach" | |||
| capabilityEBSTANonRootUser = "ebsta.non-root-user" | |||
There was a problem hiding this comment.
| capabilityEBSTANonRootUser = "ebsta.non-root-user" | |
| capabilityEBSTANonRootUser = "storage.ebs-task-volume-attach-non-root-user" |
to extend the existing capability prefix
OR
| capabilityEBSTANonRootUser = "ebsta.non-root-user" | |
| capabilityEBSTANonRootUser = "storage.ebsta-non-root-user" |
if there are restrictions on the length of the key.
agent/utils/utils.go
Outdated
| ) | ||
|
|
||
| // GenerateGIDFromPath generates a GID in range [900000, 999999] from a string | ||
| func GenerateGIDFromPath(path string) int { |
There was a problem hiding this comment.
Please put this under ecs-agent/utils: https://github.com/aws/amazon-ecs-agent/tree/master/ecs-agent/utils. This would help all agents share the same code without duplication. Please run make gomod to update go mod dependencies.
agent/utils/utils.go
Outdated
| gid := minGID + (int(num) % diff) | ||
|
|
||
| return gid | ||
| } |
There was a problem hiding this comment.
Unable to comment on the ecs-agent/daemonimages/csidriver/bin/ebs-csi-driver below. Please do not add binaries to this repo. I am not sure if this was intentional.
agent/utils/utils.go
Outdated
| minGID = 900000 | ||
| maxGID = 999999 |
There was a problem hiding this comment.
Why are the min and max GID values restricted to this range?
There was a problem hiding this comment.
This is to reduce the probability of collision with some already predefined GID.
There was a problem hiding this comment.
Thanks, please add this as a code comment
| @@ -245,9 +245,41 @@ func (d *nodeService) NodeStageVolume(ctx context.Context, req *csi.NodeStageVol | |||
| } | |||
| } | |||
| klog.V(4).InfoS("NodeStageVolume: successfully staged volume", "source", source, "volumeID", volumeID, "target", target, "fstype", fsType) | |||
|
|
|||
| const ebsPathPrefix = "/mnt/ecs/ebs/" | |||
There was a problem hiding this comment.
Is this defined anywhere else today? If yes, please reuse it, if not, lets find a place to add this. At the very least, it should be a global const in this csi-driver package.
Summary
Enables running ECS EC2 Tasks with EBS Task Attach Enabled as a Non Root User. The change allows the Non Root User inside of the running container to write to the attached EBS Volume.
Implementation details
SourceVolumeHostPath--group-addNodeStageVolume, sets the mount point ownership to this GID usingchownand sets permissions to 775 withsetgidbit usingchmodTesting
New tests cover the changes: Yes
Description for the changelog
Feature: Support EBS task attach non-root user mode
Additional Information
Does this PR include breaking model changes? If so, Have you added transformation functions? No
Does this PR include the addition of new environment variables in the README? No
Licensing
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.