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

feat: Add the additional_assume_role_principals param for cluster iam role #3282

Conversation

apetrosyan1613
Copy link

Description

Added the dynamic statement for Cluster IAM Policy to be able to expand the default Trust Relationship

Motivation and Context

Sometimes it needs to provide secure access to the cluster management via IRSA, which requires access delegation to another IAM role, which is used by the application
Example: ArgoCD store credentials for EKS cluster

Breaking Changes

No breaking changes

How Has This Been Tested?

  • I have updated at least one of the examples/* to demonstrate and validate my change(s)
  • I have tested and validated these changes using one or more of the provided examples/* projects
  • I have executed pre-commit run -a on my pull request

@apetrosyan1613 apetrosyan1613 changed the title add the additional_assume_role_principals param for cluster iam role feat:add the additional_assume_role_principals param for cluster iam role Jan 21, 2025
@apetrosyan1613 apetrosyan1613 changed the title feat:add the additional_assume_role_principals param for cluster iam role feat: add the additional_assume_role_principals param for cluster iam role Jan 21, 2025
@apetrosyan1613 apetrosyan1613 changed the title feat: add the additional_assume_role_principals param for cluster iam role feat: Add the additional_assume_role_principals param for cluster iam role Jan 21, 2025
@bryantbiggs
Copy link
Member

I'm not seeing where this is required, can you be more specific on where this requirement is coming from?

@apetrosyan1613
Copy link
Author

apetrosyan1613 commented Jan 21, 2025

I'm not seeing where this is required, can you be more specific on where this requirement is coming from?

Sure! To add the EKS cluster in ArgoCD we need the following:

  1. Create ArgocD server role with policy, which includes sts:AssumeRole action and trust relationship for ArgoCD service accounts in Kubernetes
  2. Patch the ArgoCD service account via eks annotation to use the created role
  3. Then we can create a separate role with a trust relationship to the IAM role, created before OR just update the trust relationship for the existing Cluster Management Role, which is simple, that's what this PR suggests

Then we have the cluster, which was safely added to ArgoCD, instead of using personal credentials

@apetrosyan1613
Copy link
Author

Something like this, but simplier a little bit

@bryantbiggs
Copy link
Member

sure, but I'm still not seeing where Argo needs to assume the cluster role. In addition, why utilize IRSA when EKS Pod Identity would make things a bit simpler (you don't need to know the IAM role that will be associated with the pod/namespace/service account nor do you need to add it as a label, you can create the role ahead of time and associate it with the namespace and service account and when the respective pod lands in those, it will have permissions)

@bryantbiggs
Copy link
Member

going to close this for now - I don't believe Argo needs to assume the cluster role, IRSA or Pod Identity should suffice for permissions on cluster, or cluster access entry for across clusters

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.

2 participants