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

Use eksctl when creating the IAM role and trust relationship #818

Merged
merged 1 commit into from
Sep 3, 2021
Merged

Use eksctl when creating the IAM role and trust relationship #818

merged 1 commit into from
Sep 3, 2021

Conversation

ruzickap
Copy link
Contributor

@ruzickap ruzickap commented Sep 1, 2021

Description of your changes

Simplify using IRSA by using eksctl for creating IAM role and IAM role trust relationship.

@chlunde
Copy link
Collaborator

chlunde commented Sep 1, 2021

I'm not sure if we should remove the current documentation, because not everyone uses eksctl and it is nice to show the low level AWS pieces required for IRSA, if someone wants to reproduce it using crossplane, terraform or another tool.

Maybe you can add it using <details> and <summary>?

<details>
<summary>Create IAM role with trust relationship using `eksctl`...</summary>
<p>
method one desc..
</p>

<details>
<summary>Create IAM role with trust relationship using `aws` cli...</summary>
<p>
method two desc..
</p>
</details>  
Create IAM role with trust relationship using `eksctl`...

eksctl create iamserviceaccount --cluster <cluster-name> --region <region> --name="$SERVICE_ACCOUNT_NAME" --namespace="$SERVICE_ACCOUNT_NAMESPACE" --role-name="$IAM_ROLE_NAME" --role-only --attach-policy-arn="arn:aws:iam::aws:policy/AdministratorAccess" --approve

AUTHENTICATION.md Outdated Show resolved Hide resolved
Create IAM role with trust relationship:

```
eksctl create iamserviceaccount --cluster <cluster-name> --region <region> --name="$SERVICE_ACCOUNT_NAME" --namespace="$SERVICE_ACCOUNT_NAMESPACE" --role-name="$IAM_ROLE_NAME" --role-only --attach-policy-arn="arn:aws:iam::aws:policy/AdministratorAccess" --approve

Choose a reason for hiding this comment

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

some of the variables use the <variable-name> syntax, and others use $VARIABLE_NAME. For consistency, I recommend choosing one or the other.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah... In some parts of the documentation it is being used <cluster-name> or <region> and somewhere else ${SERVICE_ACCOUNT_NAMESPACE}.

I would prefer to use bash variable like ${CLUSTER_NAME} everywhere if you agree...?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed...

AUTHENTICATION.md Outdated Show resolved Hide resolved
Signed-off-by: Petr Ruzicka <petr.ruzicka@gmail.com>
@chlunde
Copy link
Collaborator

chlunde commented Sep 3, 2021

@ruzickap looks good, I see you did some final touches, now even shellcheck would approve 😄 If you're done I think we can merge. OK?

@ruzickap
Copy link
Contributor Author

ruzickap commented Sep 3, 2021

Sure... Go ahead...

@chlunde chlunde merged commit e0259c0 into crossplane-contrib:master Sep 3, 2021
tektondeploy pushed a commit to gtn3010/provider-aws that referenced this pull request Mar 12, 2024
…ns-setup-go-digest

Update actions/setup-go digest to 93397be
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