Skip to content

Commit 7a08317

Browse files
committed
🐛: securitygroup: fix comparison of ingress rules sets.
We are comparing sets that are not compatible, so rules will always be revoked and authorized during reconciliation. We need to expand the rules from the spec so there is one rule for each item in cidrBlock/sourceSecurityGroupIds.
1 parent 888c659 commit 7a08317

File tree

1 file changed

+44
-1
lines changed

1 file changed

+44
-1
lines changed

pkg/cloud/services/securitygroup/securitygroups.go

Lines changed: 44 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -160,10 +160,12 @@ func (s *Service) ReconcileSecurityGroups() error {
160160
}
161161
current := sg.IngressRules
162162

163-
want, err := s.getSecurityGroupIngressRules(role)
163+
specRules, err := s.getSecurityGroupIngressRules(role)
164164
if err != nil {
165165
return err
166166
}
167+
// Duplicate rules with multiple cidr blocks/source security groups so that we are comparing similar sets.
168+
want := expandIngressRules(specRules)
167169

168170
toRevoke := current.Difference(want)
169171
if len(toRevoke) > 0 {
@@ -197,6 +199,47 @@ func (s *Service) ReconcileSecurityGroups() error {
197199
return nil
198200
}
199201

202+
// expandIngressRules expand the given ingress rules so that it's compatible with the list generated by
203+
// ingressRulesFromSDKType.
204+
// We assume that processIngressRulesSGs has been already called on the input, so the SourceSecurityGroupRoles have
205+
// been translated into Security Group IDs.
206+
func expandIngressRules(rules infrav1.IngressRules) infrav1.IngressRules {
207+
res := make(infrav1.IngressRules, 0, len(rules))
208+
for _, rule := range rules {
209+
base := infrav1.IngressRule{
210+
Description: rule.Description,
211+
Protocol: rule.Protocol,
212+
FromPort: rule.FromPort,
213+
ToPort: rule.ToPort,
214+
}
215+
216+
// Nothing to expand
217+
if len(rule.CidrBlocks) == 0 && len(rule.IPv6CidrBlocks) == 0 && len(rule.SourceSecurityGroupIDs) == 0 {
218+
res = append(res, base)
219+
continue
220+
}
221+
222+
for _, src := range rule.CidrBlocks {
223+
rcopy := base
224+
rcopy.CidrBlocks = []string{src}
225+
res = append(res, rcopy)
226+
}
227+
228+
for _, src := range rule.IPv6CidrBlocks {
229+
rcopy := base
230+
rcopy.IPv6CidrBlocks = []string{src}
231+
res = append(res, rcopy)
232+
}
233+
234+
for _, src := range rule.SourceSecurityGroupIDs {
235+
rcopy := base
236+
rcopy.SourceSecurityGroupIDs = []string{src}
237+
res = append(res, rcopy)
238+
}
239+
}
240+
return res
241+
}
242+
200243
func (s *Service) securityGroupIsAnOverride(securityGroupID string) bool {
201244
for _, overrideID := range s.scope.SecurityGroupOverrides() {
202245
if overrideID == securityGroupID {

0 commit comments

Comments
 (0)