Skip to content

Commit 09239b0

Browse files
committed
fix(ipset): don't strip inet6 prefixing of ipsets
The problem here stems from the fact that when netpol generates its list of expected ipsets, it includes the inet6: prefix, however, when the proxy and routing controller sent their list of expected ipsets, they did not do so. This meant that no matter how we handled it in ipset.go it was wrong for one or the other use-cases. I decided to standardize on the netpol way of sending the list of expected ipset names so that BuildIPSetRestore() can function in the same way for all invocations.
1 parent b6eedd9 commit 09239b0

File tree

4 files changed

+22
-17
lines changed

4 files changed

+22
-17
lines changed

pkg/controllers/proxy/network_services_controller.go

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -710,7 +710,10 @@ func (nsc *NetworkServicesController) syncIpvsFirewall() error {
710710

711711
setHandler.RefreshSet(serviceIPPortsSetName, serviceIPPortsIPSets[family], utils.TypeHashIPPort)
712712

713-
err := setHandler.RestoreSets([]string{localIPsIPSetName, serviceIPsIPSetName, serviceIPPortsSetName})
713+
multiFamilySetNames := utils.GenerateMultiFamilySetNames(
714+
[]string{localIPsIPSetName, serviceIPsIPSetName, serviceIPPortsSetName},
715+
)
716+
err := setHandler.RestoreSets(multiFamilySetNames)
714717
if err != nil {
715718
return fmt.Errorf("could not save ipset for service firewall: %v", err)
716719
}

pkg/controllers/routing/network_routes_controller.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -864,7 +864,8 @@ func (nrc *NetworkRoutingController) syncNodeIPSets() error {
864864

865865
ipSetHandler.RefreshSet(nodeAddrsIPSetName, currentNodeIPs[family], utils.TypeHashIP)
866866

867-
err = ipSetHandler.RestoreSets([]string{podSubnetsIPSetName, nodeAddrsIPSetName})
867+
multiFamilySetNames := utils.GenerateMultiFamilySetNames([]string{podSubnetsIPSetName, nodeAddrsIPSetName})
868+
err = ipSetHandler.RestoreSets(multiFamilySetNames)
868869
if err != nil {
869870
return fmt.Errorf("failed to sync pod subnets / node addresses ipsets: %v", err)
870871
}

pkg/utils/ipset.go

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -529,15 +529,11 @@ func scrubInitValFromOptions(options []string) []string {
529529
// add KUBE-DST-3YNVZWWGX3UQQ4VQ 100.96.1.6 timeout 0
530530
func BuildIPSetRestore(ipset *IPSet, setIncludeNames []string) string {
531531
setNames := make([]string, 0, len(ipset.sets))
532-
for setName, set := range ipset.sets {
532+
for setName := range ipset.sets {
533533
// If we've been passed a set of filter names, check to see if this set is contained within that set before
534534
// adding it to the restore to ensure that we don't impact other unrelated sets
535535
if setIncludeNames != nil {
536-
origName := setName
537-
if set.Parent.isIpv6 {
538-
origName = strings.Replace(setName, fmt.Sprintf("%s:", IPv6SetPrefix), "", 1)
539-
}
540-
if !slices.Contains(setIncludeNames, origName) {
536+
if !slices.Contains(setIncludeNames, setName) {
541537
continue
542538
}
543539
}
@@ -655,3 +651,12 @@ func (ipset *IPSet) Get(setName string) *Set {
655651
func (ipset *IPSet) Sets() map[string]*Set {
656652
return ipset.sets
657653
}
654+
655+
func GenerateMultiFamilySetNames(setNames []string) []string {
656+
multiFamilySetNames := make([]string, 0, len(setNames))
657+
for _, setName := range setNames {
658+
multiFamilySetNames = append(multiFamilySetNames, setName)
659+
multiFamilySetNames = append(multiFamilySetNames, IPSetName(setName, true))
660+
}
661+
return multiFamilySetNames
662+
}

pkg/utils/ipset_test.go

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -198,7 +198,7 @@ func Test_buildIPSetRestore_setIncludeNames(t *testing.T) {
198198
Parent: &IPSet{isIpv6: false},
199199
},
200200
}},
201-
setIncludeNames: []string{"set1", "set2"},
201+
setIncludeNames: []string{"inet6:set1", "inet6:set2"},
202202
expectedSets: []string{"inet6:set1", "inet6:set2"},
203203
excludedSets: []string{"regular-set"},
204204
},
@@ -218,7 +218,7 @@ func Test_buildIPSetRestore_setIncludeNames(t *testing.T) {
218218
Parent: &IPSet{isIpv6: true},
219219
},
220220
}},
221-
setIncludeNames: []string{"set1"},
221+
setIncludeNames: []string{"inet6:set1"},
222222
expectedSets: []string{"inet6:set1"},
223223
excludedSets: []string{"inet6:set2"},
224224
},
@@ -575,7 +575,7 @@ func Test_buildIPSetRestore_integrationRealWorldSets(t *testing.T) {
575575
Parent: &IPSet{isIpv6: true},
576576
},
577577
}},
578-
setIncludeNames: []string{"kube-router-svip", "kube-router-svip-prt"},
578+
setIncludeNames: []string{"inet6:kube-router-svip", "inet6:kube-router-svip-prt"},
579579
description: "Should match IPv6 sets by removing inet6: prefix",
580580
},
581581
{
@@ -610,15 +610,11 @@ func Test_buildIPSetRestore_integrationRealWorldSets(t *testing.T) {
610610
result := BuildIPSetRestore(tt.ipset, tt.setIncludeNames)
611611

612612
// Verify that only the specified sets are included
613-
for setName, set := range tt.ipset.sets {
613+
for setName := range tt.ipset.sets {
614614
shouldBeIncluded := false
615615
for _, includeName := range tt.setIncludeNames {
616616
// Check if this set should be included based on the filtering logic
617-
origName := setName
618-
if set.Parent != nil && set.Parent.isIpv6 {
619-
origName = strings.Replace(setName, fmt.Sprintf("%s:", IPv6SetPrefix), "", 1)
620-
}
621-
if origName == includeName {
617+
if setName == includeName {
622618
shouldBeIncluded = true
623619
break
624620
}

0 commit comments

Comments
 (0)