STOR-2766: Allow aws ebs driver to copy volumes#675
STOR-2766: Allow aws ebs driver to copy volumes#675dfajmon wants to merge 1 commit intoopenshift:mainfrom
Conversation
based on kubernetes-sigs/aws-ebs-csi-driver#2716 this is coming to OCP in openshift/aws-ebs-csi-driver#301
|
@dfajmon: This pull request references STOR-2766 which is a valid jira issue. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
WalkthroughA single AWS permission Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
@dfajmon: This pull request references STOR-2766 which is a valid jira issue. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
manifests/03_credentials_request_aws.yaml (1)
21-40:⚠️ Potential issue | 🟠 MajorScope
ec2:CopyVolumesseparately instead of inheritingresource: "*"
CopyVolumesauthorization in AWS applies to both the source volume and the copied (destination) volume. The current statement grants it withresource: "*"and no conditions, allowing the controller to copy any EBS volume visible to it. The AWS-managedAmazonEBSCSIDriverPolicyfollows a stricter approach: it scopesCopyVolumesto volume ARNs and enforces request-tag conditions requiring eitheraws:RequestTag/ebs.csi.aws.com/cluster=trueoraws:RequestTag/CSIVolumeNameto be set on the destination. This prevents unauthorized volume duplication. Please moveec2:CopyVolumesto its own statement with equivalent resource and tag-based condition constraints.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@manifests/03_credentials_request_aws.yaml` around lines 21 - 40, The IAM statement currently grants ec2:CopyVolumes under the same statement with resource: "*" — split out a new statement for the ec2:CopyVolumes action (separate from the other EC2 actions) and scope it to the relevant volume ARNs instead of "*", adding the request-tag condition keys used by AmazonEBSCSIDriverPolicy (aws:RequestTag/ebs.csi.aws.com/cluster = "true" OR aws:RequestTag/CSIVolumeName exists) so the destination copy requires the appropriate tags; update the new statement to mirror AmazonEBSCSIDriverPolicy’s resource and Condition entries for ec2:CopyVolumes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@manifests/03_credentials_request_aws.yaml`:
- Around line 21-40: The IAM statement currently grants ec2:CopyVolumes under
the same statement with resource: "*" — split out a new statement for the
ec2:CopyVolumes action (separate from the other EC2 actions) and scope it to the
relevant volume ARNs instead of "*", adding the request-tag condition keys used
by AmazonEBSCSIDriverPolicy (aws:RequestTag/ebs.csi.aws.com/cluster = "true" OR
aws:RequestTag/CSIVolumeName exists) so the destination copy requires the
appropriate tags; update the new statement to mirror AmazonEBSCSIDriverPolicy’s
resource and Condition entries for ec2:CopyVolumes.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 8ca4d1b5-47c6-4592-80d0-8501b4dbd7af
📒 Files selected for processing (1)
manifests/03_credentials_request_aws.yaml
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: dfajmon The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/lgtm |
|
@dfajmon: The following test failed, say
Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
|
/hold |
Based on kubernetes-sigs/aws-ebs-csi-driver#2716
this is coming to OCP 4.22 in openshift/aws-ebs-csi-driver#301
Summary by CodeRabbit