[DO NOT MERGE] Testing Minimal Policy#2807
[DO NOT MERGE] Testing Minimal Policy#2807mdzraf wants to merge 3 commits intokubernetes-sigs:masterfrom
Conversation
|
Adding the "do-not-merge/release-note-label-needed" label because no release-note block was detected, please follow our release note process to remove it. 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. |
Code Coverage DiffThis PR does not change the code coverage |
7275322 to
84e80d8
Compare
| "ec2:DescribeAvailabilityZones", | ||
| "ec2:DescribeInstances", | ||
| "ec2:DescribeSnapshots", | ||
| "ec2:DescribeTags", |
There was a problem hiding this comment.
We do not actually call this anywhere in the driver besides in a testing function that does not actually need it:
aws-ebs-csi-driver/pkg/cloud/cloud_test.go
Line 3666 in 33be568
So I have removed it from the policy.
| "ec2:DescribeAvailabilityZones", | ||
| "ec2:DescribeInstances", | ||
| "ec2:DescribeSnapshots", | ||
| "ec2:DescribeTags", | ||
| "ec2:DescribeVolumes", | ||
| "ec2:DescribeVolumesModifications", | ||
| "ec2:DescribeVolumeStatus" |
There was a problem hiding this comment.
These calls have * permissions because they only have ec2:Region as an allowed condition key, therefore we cannot pass in any condition key that would clearly identify a resource managed by the driver (I.e. tags on resource) see: https://docs.aws.amazon.com/service-authorization/latest/reference/list_amazonec2.html
There was a problem hiding this comment.
Only made changes here to test CI, feel free to skip review.
There was a problem hiding this comment.
Only made changes here to test CI, feel free to skip review.
docs/example-iam-policy.json
Outdated
| "Effect" : "Allow", | ||
| "Action" : [ | ||
| "ec2:AttachVolume", | ||
| "ec2:DetachVolume" | ||
| ], | ||
| "Resource" : "arn:aws:ec2:*:*:instance/*" | ||
| }, |
There was a problem hiding this comment.
Necessary for the EBS CSI Driver to perform the privileged task of attaching/detaching volumes on a host on behalf of the user.
|
/retest Trying to see if it was a flake, the pre-provisioned tests should all pass since we actually do add the required tags. |
84e80d8 to
fc23e74
Compare
fc23e74 to
6240ca8
Compare
6240ca8 to
43c91d9
Compare
|
This PR has multiple commits, and the default merge method is: merge. 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. |
|
New changes are detected. LGTM label has been removed. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
b0c8909 to
c938b47
Compare
|
/hold |
c938b47 to
234896f
Compare
What type of PR is this?
Testing CI with Minimal Policy & review new proposed minimal policy.
Manual Tests Completed: