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

Add eks.amazonaws.com/role-arn annotation #46

Closed
wants to merge 4 commits into from
Closed

Conversation

DmitryDodzin
Copy link
Member

@DmitryDodzin DmitryDodzin commented Mar 7, 2024

Add sa.roleArn value that maps to eks.amazonaws.com/role-arn annotation on service account.

relevant for metalbear-co/mirrord#2293.

Copy link
Member

@meowjesty meowjesty left a comment

Choose a reason for hiding this comment

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

👍

@DmitryDodzin DmitryDodzin marked this pull request as draft March 7, 2024 17:14
mirrord-operator/README.md Outdated Show resolved Hide resolved

aws iam create-role --role-name mirrord-operator-role --assume-role-policy-document file://trust-relationship.json --description "Role for SQS splitting for mirrord-operator"

aws iam attach-role-policy --role-name mirrord-operator-role --policy-arn=arn:aws:iam::$account_id:policy/mirrord-operator-role
Copy link
Member

Choose a reason for hiding this comment

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

When I ran this I got

An error occurred (NoSuchEntity) when calling the AttachRolePolicy operation: Policy arn:aws:iam::<redacted_by_t4lz>:policy/mirrord-operator-role does not exist or is not attachable.

So for my testing I just attached mirrorrd-operator-role to the preexisting AmazonSQSFullAccess policy (with ARN arn:aws:iam::aws:policy/AmazonSQSFullAccess). I don't know if that's a valid solution for our users as well, or if the limitation to a specific account id in our custom policy is important.

@aviramha
Copy link
Member

@Razz4780 @DmitryDodzin @t4lz do we need this?

@Razz4780
Copy link
Contributor

@Razz4780 @DmitryDodzin @t4lz do we need this?

Don't think so, I've seen arn role both in mirrord operator setup and in the new chart. @t4lz ? 👀

@DmitryDodzin
Copy link
Member Author

@Razz4780 @DmitryDodzin @t4lz do we need this?

Well we can slim up the doc section because it's mostly copy-paste from aws-docs but the .Values.sa.roleArn can be useful in CI setting instead of introducing outside patch to link the arn

@t4lz
Copy link
Member

t4lz commented Aug 19, 2024

I based #84 on this branch, so these commits are already merged (but I did change the README).
I am confused as to why the diff still shows stuff that's already in main.

@t4lz t4lz closed this Aug 19, 2024
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.

5 participants