-
Notifications
You must be signed in to change notification settings - Fork 45
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
Fix nil pointer error when DescribeSecurityGroups is not possible dur… #235
Fix nil pointer error when DescribeSecurityGroups is not possible dur… #235
Conversation
…ing clean up of resources
@luis-falcon: all tests passed! Full PR test history. Your PR dashboard. 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 kubernetes-sigs/prow repository. I understand the commands that are listed here. |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #235 +/- ##
==========================================
+ Coverage 15.24% 22.94% +7.70%
==========================================
Files 17 8 -9
Lines 1358 902 -456
==========================================
Hits 207 207
+ Misses 1137 681 -456
Partials 14 14
|
/lgtm |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested and ensured reachability of
- "Unable to describe security groups" error when
ec2:DescribeSecurityGroups
is denied by IAM - "Unable to detach instance from security group" error when
ec2:ModifyInstanceAttribute
is denied by IAM ModifyInstanceAttribute
when all actions allowed by IAM
Note that I was only able to reach Case 1 when specifying an existing security group a (i.e., when the verifier is not asked to create a temporary security group). This is because the temporary security group verification process will fail if ec2:DescribeSecurityGroups
is denied by IAM, preventing us from ever reaching any of the code touched by this PR. I believe OCM always uses ForceTempSecurityGroup
, so it's strange that they're seeing this error. Perhaps customers have some very arcane IAM rules in place that are allowing the first ec2:DescribeSecurityGroups
call to go through (during temp. SG creation) while denying later calls to the same API endpoint 🤷🏻
Regardless of the above caveat, approving because the bug is still reproducible in some cases, and this fix works in those cases. Nice work @luis-falcon 🙌🏻
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: abyrne55 The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Fix nil pointer error when DescribeSecurityGroups is not possible during clean up of resources
-Bug fix
What does this PR do? / Related Issues / Jira
fixes issue with nil pointer when trying to iterate through a list of Security groups. We get this list with a DescribesecurityGroups call but if this fails the variable had a nil pointer.
To Fix I made a conditional to only execute the code detaching the SG from the instance only if we get a result from the DescribeSecurityGroup api Call.
Checklist
Reviewer's Checklist