Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Privileged tasks on AWS #2413

Closed
wants to merge 5 commits into from
Closed

Privileged tasks on AWS #2413

wants to merge 5 commits into from

Conversation

manuelesimi
Copy link
Collaborator

As part of our HIPAA compliance procedure (https://aws.amazon.com/compliance/hipaa-compliance/), we are requested to be able to set elevated privileges on the tasks we register on AWS.

This PR extends the AWS configuration in Nextflow to allow setting such privileges on the host container instances. This mirrors a similar behavior NF already has for Azure (#2157).

Here's a sample configuration:

       process {
            executor = 'awsbatch'
            container =  '...'
            queue = '...'
        }
        aws {
            batch {
                privileged = true
                cliPath = '/home/ec2-user/miniconda/bin/aws'
            }
            region = 'us-east-1'
        }

See AWS doc: https://docs.aws.amazon.com/AmazonECS/latest/developerguide/task_definition_parameters.html

Signed-off-by: Manuele Simi <manuele.simi@gmail.com>
…assumption).

Signed-off-by: Manuele Simi <manuele.simi@gmail.com>
Signed-off-by: Manuele Simi <manuele.simi@gmail.com>
docs/config.rst Outdated Show resolved Hide resolved
Co-authored-by: Abhinav Sharma <abhi18av@users.noreply.github.com>
@abhi18av
Copy link
Member

Hi @manuelesimi , thanks for this PR!

I was wondering, given that a previous PR covered something similar for Azure #2157 , specifically adding the priviledged as well as runAs for the Azure executor.

azure.batch.pools.<name>.privileged             Enable the task to run with elevated access. Ignored if `runAs` is set (default: ``false``).
azure.batch.pools.<name>.runAs                  Specify the username under which the task is run. The user must already exist on each node of the pool.

I was wonrdering it makes sense to address the user field for AWS as well within the scope of this PR?

@manuelesimi
Copy link
Collaborator Author

@abhi18av Yes, it would make sense to have a similar runAs option and I can definitely add it.

Right now my problem is to fix the sign off on my commits. Somehow git used my other email address (well, it's the default, by I had the repo configured for my gmail address), so the DCO check fails. I followed the instructions to fix it... but no luck. Any suggestion?

@abhi18av
Copy link
Member

abhi18av commented Oct 27, 2021

That's an interesting problem :)

I have not ran across this myself, but this is what I'd do.

( Best to make a copy of the aws/privileged_tasks branch prior to doing this. )

@manuelesimi
Copy link
Collaborator Author

Thanks for the suggestion. The problem is that I merged back the master branch into this one, and the last 5 commits are not the ones with the changes proposed here (I branched for this 2 weeks ago, then got swamped in something else).

I know how to change the author of past commits with rebase + amend, but for the same reason mentioned above, rebase wants me to merge after each amendment.

I believe the fastest way to fix this is to branch again from master and applies the changes proposed in this commit. I'm closing this PR (since we can't change the source branch) and open a new one when the new branch is ready.

@manuelesimi manuelesimi deleted the aws/privileged_tasks branch October 27, 2021 15:12
@pditommaso
Copy link
Member

Several problems with this feature:

  • it defined in the batch scope and therefore applies to all jobs. More likely it's expected to be used as task level
  • the privileged flag should be reflected in the unique hash created for the job request
  • it overlaps with this feature Support docker container option --shm-size, per process #2282 which more generically should provide the ability to inject any containerOptions in corresponding job definition.

Also, how is the privileged flag related to HIPAA compliance ? I would have expected the opposite.

@manuelesimi
Copy link
Collaborator Author

I see the overlapping with #2282.
Then, I'm closing this one (well, I already did for other reasons...) and wait for the new and more generic feature on the container options.

For HIPAA compliance, we have a rule requesting to have non-privileged permissions on the task, so we must be able to explicitly set them to false. I see that my initial comment was misleading in this sense, but not relevant for the PR).

Thanks!

@pditommaso
Copy link
Member

Oh, so the requirement is to explicitly set to false .. (funny)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants