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

[Workflows-549] remove iam users #312

Open
wants to merge 2 commits into
base: dev
Choose a base branch
from
Open

Conversation

danlu1
Copy link
Contributor

@danlu1 danlu1 commented Jun 11, 2024

Problem:

The AWS credentials can't be populated with IAM role arn. And get_secret_value function does not work with IAM role arn. Ideally, after converting to using IAM role, we no longer need to use accessKeys as the AWS credential of the Tower workspaces. Instead, we use AWS arn (see this instruction)

Solution:

As the Seqera's instruction, an instance profile should be created and attached with the IAM role. My understanding is that creating InstanceProfile for the role would allow EC2 to receive the IAM role credential when it assumes the role.

Testing:
The credential can be saved in Tower right now, but another error: {'message': "Cannot determine region of bucket 'example-dev-project-tower-scratch'"} popped up when generating the computation environment. The errors lie in these lines

@danlu1 danlu1 requested a review from a team as a code owner June 11, 2024 15:51
Copy link

sonarcloud bot commented Jun 11, 2024

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

Copy link
Contributor

@zaro0508 zaro0508 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@danlu1 can you please explain in this PRs description why you have taken this approach to fixing the problem and why you think this will fix it?

@thomasyu888
Copy link
Collaborator

@danlu1 can you please explain in this PRs description why you have taken this approach to fixing the problem and why you think this will fix it?

@zaro0508 we actually aren't sure what is going on. @BWMac / @danlu1 can you fill @zaro0508 in? Maybe he can also help us!

@danlu1
Copy link
Contributor Author

danlu1 commented Jun 12, 2024

@zaro0508 I would appreciate your insights on these changes (this PR along with its prior PR).

Based on my preliminary research, the IAM role for Seqera, named TowerRole, has been added in your PR. The required IAM role policy (nextflow-launch-iam-policy and nextflow-forge-iam-policy) and trust policy (in the AssumeRolePolicyDocument component of the TowerRole) are in place.

As Seqera's instruction regarding creating IAM artifacts, I submitted this PR by adding a TowerRoleProfile and attach the TowerRole to it. After creating the instance profile, EC2 should be able to receive the IAM role credential when it assumes the TowerRole. I tested this PR on my local and successfully added the aws credential in the Tower-dev project.

Even though the credential has been added, another error: {'message': "Cannot determine region of bucket 'example-dev-project-tower-scratch'"}(the bucket is the workDir of the Tower project) occurs when I tried to generate the computation environment for tower using the configure-tower-projects.py , and the errors lie in these lines. This confused me a lot since TowerRole has been added to Bucket policy and has GetBucketLocation permission.

Can you enlighten me on what could cause the issue? Also, can you share more on how you expect the EcsServiceRole to come into play? Thanks in advance.

data = {
"credentials": {
"name": self.stack_name,
"provider": "aws",
"keys": {
"accessKey": credentials["aws_access_key_id"],
# "accessKey": credentials["aws_access_key_id"],
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i believe the key to setting this key is to look at line #544, response = self.tower.request("POST", endpoint, params=params, json=data). That is using the seqera tower credentials API to Creates new Tower credentials. According to the API docs, passing in an assumeRoleArn also requires accessKey and secretKey therefore this will not work.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, this is the confusing part I have. I remembered it worked when keeping the accessKey and secretKey. However, as my understanding based on this doc, the accessKey and secretKey should be no longer needed if implementing role based authentication. Instead, the AWS role arn is used after the IAM artifacts is created and new credentials are added in Seqera.

Copy link
Contributor

@zaro0508 zaro0508 Nov 1, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately this deployment setup does not fit with the assume role use case that's explained in the seqera docs. This script configure-tower-projects.py is executed with a Github action which means it is running in a Github supplied instance, NOT an AWS instance. Only AWS instances are setup to assume roles that give access to the Seqera app (and APIs).

Also the doc you referenced says AWS-based customers can configure Seqera Platform to authenticate to AWS services like Batch with an IAM Role instead of IAM user credentials which means that setup allows Seqera access to access AWS. However this script is doing a POST request to Seqera which means it needs access to Seqera. There is no way to allow a Github instances to assume a role to access Seqera unless Seqera allows setting up a trust relationship (via OIDC) between the two entities.

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.

3 participants