provide better examples for setting up IAM roles#317
provide better examples for setting up IAM roles#317radoslawszulgo wants to merge 11 commits intomainfrom
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR significantly enhances the documentation for setting up IAM roles for Percona Backup for MongoDB in AWS environments. It expands the content from basic instructions to comprehensive, step-by-step guides with concrete examples.
Key Changes:
- Restructured EC2 instance IAM section to focus on role assumption with detailed policy examples
- Expanded IRSA (IAM Roles for Service Accounts) section with complete setup instructions including OIDC provider configuration, policy creation, and service account annotation
- Added extensive JSON policy examples for trust relationships and S3 permissions
- Included practical bash commands for AWS CLI operations
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
docs/manage/automate-s3-access.md
Outdated
| { | ||
| "Effect": "Allow", | ||
| "Principal": { | ||
| "AWS": "arn:aws:iam::EC2_ACCOUNT_ID:role/pbm-target-role" |
There was a problem hiding this comment.
The role name in the trust policy example is incorrect. The trust policy is for the target role that will be assumed, but line 40 shows pbm-target-role which is the target role itself. The trust policy should reference the EC2 instance role that is allowed to assume this target role, not the target role itself.
The Principal.AWS field should specify the EC2 instance role ARN (e.g., arn:aws:iam::EC2_ACCOUNT_ID:role/pbm-ec2-instance-role), not pbm-target-role.
There was a problem hiding this comment.
@copilot open a new pull request to apply changes based on this feedback
docs/manage/automate-s3-access.md
Outdated
| ] | ||
| } | ||
| ``` | ||
| > Remember to replace `EC2_ACCOUNT_ID` and `pbm-target-role` with the account ID and role name of your EC2 instance. |
There was a problem hiding this comment.
The guidance text at line 47 is misleading. It states to "replace EC2_ACCOUNT_ID and pbm-target-role", but pbm-target-role in line 40 should actually be replaced with the EC2 instance role name, not the target role name. The note should clarify that users need to replace these placeholders with their EC2 instance's role information, not the target role being created.
| > Remember to replace `EC2_ACCOUNT_ID` and `pbm-target-role` with the account ID and role name of your EC2 instance. | |
| > Remember to replace `EC2_ACCOUNT_ID` and `pbm-target-role` with the account ID and role name of your EC2 instance's IAM role (the role attached to your EC2 instance that will assume the target role). Do **not** use the target role's name here. |
docs/manage/automate-s3-access.md
Outdated
|
|
||
| * **Trust Policy**: The trust policy of the target role must allow the EC2 instance's role to assume it. | ||
|
|
||
| For example, if your EC2 instance role is `arn:aws:iam::EC2_ACCOUNT_ID:role/pbm-target-role`, use the following trust policy for your target role: |
There was a problem hiding this comment.
The introductory text for the trust policy is confusing. Line 31 states "if your EC2 instance role is arn:aws:iam::EC2_ACCOUNT_ID:role/pbm-target-role", but this should refer to the EC2 instance role (e.g., pbm-ec2-instance-role), not pbm-target-role. Using the same role name for both the EC2 instance role and the target role creates confusion about which role is which.
| First, create a trust policy JSON file (e.g., `pbm-trust-policy.json`). This policy allows your Kubernetes service account to assume the role. Replace `<account-id>`, `<region>`, and `<oidc-id>` with your AWS account ID, EKS cluster region, and the OIDC ID from step 1. | ||
| ```json | ||
| { | ||
| "Version": "2012-10-17", | ||
| "Statement": [ | ||
| { | ||
| "Effect": "Allow", | ||
| "Principal": { | ||
| "Federated": "arn:aws:iam::<account-id>:oidc-provider/oidc.eks.<region>.amazonaws.com/id/<oidc-id>" |
There was a problem hiding this comment.
Missing context for <oidc-id> placeholder. While the text at line 163 mentions "the OIDC ID from step 1", step 1 shows how to get the OIDC issuer URL but doesn't explicitly explain that the OIDC ID is the unique identifier at the end of that URL. Consider adding a note explaining how to extract the <oidc-id> from the issuer URL returned in step 1.
There was a problem hiding this comment.
@copilot open a new pull request to apply changes based on this feedback
|
@radoslawszulgo I've opened a new pull request, #318, to work on those changes. Once the pull request is ready, I'll request review from you. |
Co-authored-by: radoslawszulgo <175048287+radoslawszulgo@users.noreply.github.com>
Co-authored-by: radoslawszulgo <175048287+radoslawszulgo@users.noreply.github.com>
Co-authored-by: radoslawszulgo <175048287+radoslawszulgo@users.noreply.github.com>
Add OIDC ID extraction guidance for IRSA setup
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
@radoslawszulgo I've opened a new pull request, #319, to work on those changes. Once the pull request is ready, I'll request review from you. |
…rget role Co-authored-by: radoslawszulgo <175048287+radoslawszulgo@users.noreply.github.com>
Fix IAM trust policy example to reference EC2 instance role
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 1 out of 2 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| ```bash | ||
| export AWS_ROLE_ARN=arn:aws:iam::ACCOUNT_ID:role/pbm-target-role | ||
| export AWS_SESSION_NAME=pbm-session-$(hostname) |
There was a problem hiding this comment.
The AWS environment variable name AWS_SESSION_NAME should be AWS_ROLE_SESSION_NAME according to AWS SDK documentation. The correct environment variable for specifying the session name when assuming a role is AWS_ROLE_SESSION_NAME, not AWS_SESSION_NAME.
| export AWS_SESSION_NAME=pbm-session-$(hostname) | |
| export AWS_ROLE_SESSION_NAME=pbm-session-$(hostname) |
| "s3:DeleteObject", | ||
| "s3:GetBucketLocation" | ||
| ], | ||
| "Resource": "arn:aws:s3:::your-pbm-bucket/*" | ||
| }, | ||
| { | ||
| "Effect": "Allow", | ||
| "Action": "s3:ListBucket", |
There was a problem hiding this comment.
The permissions in the S3 policy may be insufficient. The policy includes s3:GetBucketLocation but is missing s3:ListBucket permissions on the same resource level. Line 63 has s3:GetBucketLocation in the array with object-level permissions (arn:aws:s3:::your-pbm-bucket/*), but GetBucketLocation is a bucket-level operation and should be applied to arn:aws:s3:::your-pbm-bucket (without the /*). Consider moving s3:GetBucketLocation to be with s3:ListBucket on lines 69-70, or add it as a separate statement.
| "s3:DeleteObject", | |
| "s3:GetBucketLocation" | |
| ], | |
| "Resource": "arn:aws:s3:::your-pbm-bucket/*" | |
| }, | |
| { | |
| "Effect": "Allow", | |
| "Action": "s3:ListBucket", | |
| "s3:DeleteObject" | |
| ], | |
| "Resource": "arn:aws:s3:::your-pbm-bucket/*" | |
| }, | |
| { | |
| "Effect": "Allow", | |
| "Action": [ | |
| "s3:ListBucket", | |
| "s3:GetBucketLocation" | |
| ], |
| "s3:DeleteObject", | ||
| "s3:GetBucketLocation" | ||
| ], | ||
| "Resource": "arn:aws:s3:::your-pbm-bucket/*" | ||
| }, | ||
| { | ||
| "Effect": "Allow", | ||
| "Action": "s3:ListBucket", |
There was a problem hiding this comment.
The permissions in the S3 policy may be insufficient. The policy includes s3:GetBucketLocation in the object-level permissions (line 141: arn:aws:s3:::your-pbm-bucket/*), but GetBucketLocation is a bucket-level operation and should be applied to arn:aws:s3:::your-pbm-bucket (without the /*). Consider moving s3:GetBucketLocation to be with s3:ListBucket on lines 147-148, or add it as a separate statement for the bucket resource.
| "s3:DeleteObject", | |
| "s3:GetBucketLocation" | |
| ], | |
| "Resource": "arn:aws:s3:::your-pbm-bucket/*" | |
| }, | |
| { | |
| "Effect": "Allow", | |
| "Action": "s3:ListBucket", | |
| "s3:DeleteObject" | |
| ], | |
| "Resource": "arn:aws:s3:::your-pbm-bucket/*" | |
| }, | |
| { | |
| "Effect": "Allow", | |
| "Action": [ | |
| "s3:ListBucket", | |
| "s3:GetBucketLocation" | |
| ], |
| Then, annotate the service account. Replace `<namespace>`, `<service-account-name>`, and `<role_arn>` with the correct values. | ||
| ```bash | ||
| kubectl annotate serviceaccount <service-account-name> -n <namespace> \ | ||
| eks.amazonaws.com/role-arn="<role_arn>" |
There was a problem hiding this comment.
[nitpick] Inconsistent placeholder format. Throughout the document, placeholders use different formats: <cluster_name> (line 119), <region> and <cluster-name> (line 123), <account-id>, <region>, and <oidc-id> (line 163), <namespace> and <service-account-name> (lines 180, 187), and <role_arn> (line 213). Consider using a consistent format for all placeholders, either all with hyphens (e.g., <cluster-name>) or all with underscores (e.g., <cluster_name>).
| Then, annotate the service account. Replace `<namespace>`, `<service-account-name>`, and `<role_arn>` with the correct values. | |
| ```bash | |
| kubectl annotate serviceaccount <service-account-name> -n <namespace> \ | |
| eks.amazonaws.com/role-arn="<role_arn>" | |
| Then, annotate the service account. Replace `<namespace>`, `<service-account-name>`, and `<role-arn>` with the correct values. | |
| ```bash | |
| kubectl annotate serviceaccount <service-account-name> -n <namespace> \ | |
| eks.amazonaws.com/role-arn="<role-arn>" |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
No description provided.