-
Notifications
You must be signed in to change notification settings - Fork 10
feat: add security group drift detection #467
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
base: main
Are you sure you want to change the base?
feat: add security group drift detection #467
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: meomnzak 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 |
|
|
||
| // createTestNodeClassWithPlacementStrategy creates a NodeClass using PlacementStrategy | ||
| // instead of explicit Subnet - used for testing subnet pool drift scenarios | ||
| func (s *E2ETestSuite) createTestNodeClassWithPlacementStrategy(t *testing.T, testName string) *v1alpha1.IBMNodeClass { |
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.
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.
yeaaaaa fixing e2e properly has been a moving target for a while now... the latest open issue to track this is #431
I'm putting this off until we get new infra. pls leave the e2e files as they are for now, and feel free to ignore the failures for now as well lol
| } else { | ||
| // Get default security group for VPC | ||
| defaultSG, sgErr := p.getDefaultSecurityGroup(ctx, vpcClient, nodeClass.Spec.VPC) | ||
| defaultSG, sgErr := vpcClient.GetDefaultSecurityGroup(ctx, nodeClass.Spec.VPC) |
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.
I am not sure how costly this vpc call is. an alternative will be adding Status.SecurityGroups and populate it. Your call
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #467 +/- ##
==========================================
- Coverage 55.77% 55.49% -0.28%
==========================================
Files 58 58
Lines 8757 8792 +35
==========================================
- Hits 4884 4879 -5
- Misses 3525 3563 +38
- Partials 348 350 +2
🚀 New features to boost your workflow:
|
c6f271c to
acf1d57
Compare

Description
Add security group drift detection
Type of change
Testing
helm lintand template validation)Checklist
Additional context
closes #442